Closed
Bug 766713
Opened 12 years ago
Closed 12 years ago
IonMonkey: (ARM) LoadValue does not handle all of the cases that it should.
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: mjrosenb, Unassigned)
Details
Attachments
(1 file)
2.93 KB,
patch
|
jbramley
:
review+
|
Details | Diff | Splinter Review |
This is for the case that loadvalue is given a BaseIndex, which is for operations of the form x = *(ra + rb << c + #d) When this code was written, it was assumed that #d would always be 0, but since then, that invariant has varied. Simple fix for the issue is attached.
Attachment #635062 -
Flags: review?(Jacob.Bramley)
Comment 1•12 years ago
|
||
Comment on attachment 635062 [details] [diff] [review] /home/mrosenberg/patches/improveLoadValue-r0.patch Review of attachment 635062 [details] [diff] [review]: ----------------------------------------------------------------- The implementation is correct, as far as I can see. I suggested a simplification, but the patch is good whether you choose to do that or not. ::: js/src/ion/arm/MacroAssembler-arm.cpp @@ +2383,1 @@ > } Can't the whole thing be reduced in the same way? The simpler loadValue will try to use ldrd anyway, so you only need two lines for the whole function, I think: ma_alu(base, lsl(index, TimesEight), ScratchRegister, op_add); loadValue(Address(ScratchRegister, off.value), val); The only difference is where the address calculation is done in the case where off.value is 0. Now: lsl ScratchRegister, index, TimesEight ldrd val.payloadReg(), val.typeReg(), [base, ScratchRegister] Simplified: add ScratchRegister, base, index<<TimesEight ldrd val.payloadReg(), val.typeReg(), [ScratchRegister]
Attachment #635062 -
Flags: review?(Jacob.Bramley) → review+
Reporter | ||
Comment 2•12 years ago
|
||
landed: http://hg.mozilla.org/projects/ionmonkey/rev/c8ce2a0acddb
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
•