The default bug view has changed. See this FAQ.

IonMonkey: compile global scripts

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jandem, Assigned: dvander)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
We need this for bitwise-and.
(Reporter)

Updated

5 years ago
Assignee: general → dvander
Status: NEW → ASSIGNED
(Assignee)

Comment 1

5 years ago
Created attachment 587571 [details] [diff] [review]
WIP v0, runs bitwise-and
(Assignee)

Comment 2

5 years ago
Created attachment 587581 [details] [diff] [review]
WIP v1, handles bailouts
Attachment #587571 - Attachment is obsolete: true
(Assignee)

Comment 3

5 years ago
Created attachment 587585 [details] [diff] [review]
patch

passes tests
Attachment #587581 - Attachment is obsolete: true
Attachment #587585 - Flags: review?
(Assignee)

Updated

5 years ago
Attachment #587585 - Flags: review? → review?(christopher.leary)
Comment on attachment 587585 [details] [diff] [review]
patch

Review of attachment 587585 [details] [diff] [review]:
-----------------------------------------------------------------

Just a few nits. Looks good!

::: js/src/ion/Bailouts.cpp
@@ +259,5 @@
>          return BAILOUT_RETURN_FATAL_ERROR;
>      activation->setBailout(br);
>  
>      // Non-function frames are not supported yet. We don't compile or enter
>      // global scripts so this assert should not fire yet.

Nit: can remove this comment, because we do!

::: js/src/ion/CompileInfo.h
@@ +46,5 @@
>  
>  inline uintN
>  CountArgSlots(JSFunction *fun)
>  {
> +    return fun ? fun->nargs + 2 : 1; // +2 for |scopeChain| and |this|, or +1 for |this|

As discussed on IRC, I think the global object gets +1 for |scopeChain| instead of |this|

::: js/src/ion/IonBuilder.cpp
@@ +2444,5 @@
>          osrBlock->add(scopev);
>          osrBlock->initSlot(slot, scopev);
>      }
>  
>      // Initialize |this| parameter.

Nit: can we push this comment down into the block?

::: js/src/jit-test/tests/v8-v5/check-splay.js
@@ +91,5 @@
>    } while (splayTree.find(key) != null);
>    splayTree.insert(key, GeneratePayloadTree(kSplayTreePayloadDepth, key));
>    return key;
>  }
> +dis(InsertNewNode);

Nit: not sure if it's bad to leave this in.
Attachment #587585 - Flags: review?(christopher.leary) → review+
(Assignee)

Comment 5

5 years ago
http://hg.mozilla.org/projects/ionmonkey/rev/f3fef5d48874
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 776361
You need to log in before you can comment on or make changes to this bug.