IonMonkey: Compile JSOP_DUP2

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: nbp, Assigned: jandem)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

6 years ago
Necessary for benchmarks.
(Assignee)

Comment 1

6 years ago
Created attachment 576939 [details] [diff] [review]
Compile dup and dup2

dup2 is the last op we need for bitops-nsieve-bits (on top of getelem/setelem, and after working around missing OSR).
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Attachment #576939 - Flags: review?(sstangl)
Comment on attachment 576939 [details] [diff] [review]
Compile dup and dup2

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

::: js/src/ion/IonBuilder.cpp
@@ +680,5 @@
>        case JSOP_SETELEM:
>          return jsop_setelem();
>  
> +      case JSOP_DUP:
> +        current->push(current->peek(-1));

Duplication must ensure that StackSlot::copyOf is correctly set: since JSOP_DUP copies a value on the stack, we actually want a function that appends to the linked list of copies.

pushCopy(uint32 slot) has a prototype in MIRGraph.h but no implementation -- maybe a relic. Would you mind removing it as part of this patch?

pushVariable(uint32 slot) looks like what we want.

@@ +1689,5 @@
> +{
> +    MDefinition *lhs = current->peek(-2);
> +    MDefinition *rhs = current->peek(-1);
> +    current->push(lhs);
> +    current->push(rhs);

Needs the same change to respect copies.
Attachment #576939 - Flags: review?(sstangl)
(Assignee)

Comment 3

6 years ago
Created attachment 577214 [details] [diff] [review]
Patch v2

Thanks. This patch uses pushSlot, since pushVariable is private (and pushSlot calls pushVariable).
Attachment #576939 - Attachment is obsolete: true
Attachment #577214 - Flags: review?(sstangl)
(Assignee)

Comment 4

6 years ago
Created attachment 577216 [details] [diff] [review]
Patch v2

Make the assert a bit stronger. Sorry for the bugspam.
Attachment #577214 - Attachment is obsolete: true
Attachment #577214 - Flags: review?(sstangl)
Attachment #577216 - Flags: review?(sstangl)

Updated

6 years ago
Attachment #577216 - Flags: review?(sstangl) → review+
(Assignee)

Comment 5

6 years ago
Pushed:

http://hg.mozilla.org/projects/ionmonkey/rev/7665e358c6a2
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.