Closed
Bug 763883
Opened 12 years ago
Closed 12 years ago
IonMonkey: (ARM) -x does not deal with INT_MIN properly
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: mjrosenb, Unassigned)
Details
Attachments
(1 file, 1 obsolete file)
2.90 KB,
patch
|
jbramley
:
review+
|
Details | Diff | Splinter Review |
It currently produces INT_MIN rather than -INT_MIN.
Attachment #632205 -
Flags: review?(Jacob.Bramley)
Reporter | ||
Comment 1•12 years ago
|
||
with more qrefresh
Attachment #632205 -
Attachment is obsolete: true
Attachment #632205 -
Flags: review?(Jacob.Bramley)
Attachment #632206 -
Flags: review?(Jacob.Bramley)
Comment 2•12 years ago
|
||
Comment on attachment 632206 [details] [diff] [review] /home/mrosenberg/patches/fixNegate-r1.patch Review of attachment 632206 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the simple nits fixed. ::: js/src/ion/arm/CodeGenerator-arm.cpp @@ +350,5 @@ > { > const LAllocation *lhs = ins->getOperand(0); > const LAllocation *rhs = ins->getOperand(1); > const LDefinition *dest = ins->getDef(0); > + fprintf(stderr, "subtracting...\n"); This is surely debug code! @@ +388,5 @@ > + if (mul->canOverflow()) { > + masm.ma_cmp(ToRegister(dest), Imm32(INT_MIN)); > + c = Assembler::Equal; > + } > + masm.ma_rsb(ToRegister(lhs), Imm32(0), ToRegister(dest)); Oh, it's simpler than that. I don't know why I didn't think of it when we discussed this on IRC, but you can simply do this: rsbs dest, lhs, #0 bvs bailout That is, the operation dest=0-lhs results in (signed) overflow iff lhs=INT_MIN, so you don't need a separate comparison.
Attachment #632206 -
Flags: review?(Jacob.Bramley) → review+
Reporter | ||
Comment 3•12 years ago
|
||
Landed: http://hg.mozilla.org/projects/ionmonkey/rev/15afceb8e292
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•