Closed Bug 678625 Opened 13 years ago Closed 13 years ago

IonMonkey: GVN constant folding does not respect integer overflow rules

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: adrake, Assigned: rpearl)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached file Test case
Attached test case prints -2147483648 when GVN is enabled, and should print 2147483648.
Attached patch patch v0Splinter Review
This is a big refactor, which tries to pull out a lot of common functionality. 

We respect integer overflow rules implicitly, by using the built-in setNumber() functionality and checking whether the types change. Unfortunately, we can't promote to a double if this sort of overflow happens, due to needing to re-analyze types. We are, however, guaranteed that we will bailout if this does happen, and we can even tell ahead of time, now...

Thoughts?
Assignee: general → rpearl
Status: NEW → ASSIGNED
Attachment #553337 - Flags: review?(dvander)
Comment on attachment 553337 [details] [diff] [review]
patch v0

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

Cool, this seems a lot cleaner.

::: js/src/ion/MIR.cpp
@@ +525,5 @@
> +MDefinition *
> +MBinaryArithInstruction::foldsTo(bool useValueNumbers)
> +{
> +    if (specialization_ != MIRType_Double && specialization_ != MIRType_Int32)
> +        return this;

You can simplify this to (specialization_ == MIRType_None)
Attachment #553337 - Flags: review?(dvander) → review+
http://hg.mozilla.org/projects/ionmonkey/rev/642b96936887
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.