Closed Bug 707926 Opened 13 years ago Closed 13 years ago

IonMonkey: Compile JSOP_POS

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: ejpbruel, Assigned: ejpbruel)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch Implemented JSOP_POS (obsolete) — Splinter Review
Here's my first proposal. Running jstests.py with --ion and --ion-eager flags show no regressions relative to the parent commit.
Attachment #579296 - Flags: review?(jdemooij)
Comment on attachment 579296 [details] [diff] [review] Implemented JSOP_POS Review of attachment 579296 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks for taking this. You should add some tests to jit-tests/tests/ion/ though - something like testSubtract.js. ::: js/src/ion/IonBuilder.cpp @@ +1955,5 @@ > +{ > + TypeOracle::Unary types = oracle->unaryOp(script, pc); > + MDefinition *value = current->pop(); > + MInstruction *ins; > + if (types.ival == MIRType_Int32) This has to be types.rval, I think. If the argument is a string like "123", ival is MIRType_String and rval is MIRType_Int32 so we want to use ToInt32. If at some point the string becomes "123.4", MToInt32 will bailout (since 123.4 does not fit in an int), |double| will be added to the result typeset of this JSOP_POS and the next time we compile the function types.rval is MIRType_Double and we will choose ToDouble. This may not work yet though because we need on-stack invalidation (bug 686927).
Attachment #579296 - Flags: review?(jandemooij) → review+
Status: NEW → ASSIGNED
I've made the changes requested by jandem, but since I don't have L2 commit rights yet I cannot push the patch myself. Instead, I'm attaching it here. jandem: if you would be so kind to commit this patch to the trunk, I think we can consider this bug resolved.
Attachment #579296 - Attachment is obsolete: true
Attachment #581656 - Flags: review?(jandemooij)
Attachment #581656 - Flags: review?(jandemooij) → review+
Pushed: http://hg.mozilla.org/projects/ionmonkey/rev/885782dc302a Thanks again for taking this.
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.

Attachment

General

Created:
Updated:
Size: