Closed Bug 701956 Opened 13 years ago Closed 13 years ago

IonMonkey: Compile JSOP_DUP2

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: nbp, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

Necessary for benchmarks.
Attached patch Compile dup and dup2 (obsolete) — Splinter Review
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)
Attached patch Patch v2 (obsolete) — Splinter Review
Thanks. This patch uses pushSlot, since pushVariable is private (and pushSlot calls pushVariable).
Attachment #576939 - Attachment is obsolete: true
Attachment #577214 - Flags: review?(sstangl)
Attached patch Patch v2Splinter Review
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)
Attachment #577216 - Flags: review?(sstangl) → review+
Pushed:

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