Closed
Bug 707926
Opened 13 years ago
Closed 13 years ago
IonMonkey: Compile JSOP_POS
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: ejpbruel, Assigned: ejpbruel)
References
Details
Attachments
(1 file, 1 obsolete file)
3.31 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•13 years ago
|
||
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 2•13 years ago
|
||
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+
Updated•13 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•13 years ago
|
||
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)
Updated•13 years ago
|
Attachment #581656 -
Flags: review?(jandemooij) → review+
Comment 4•13 years ago
|
||
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.
Description
•