Closed Bug 893543 Opened 6 years ago Closed 6 years ago

Ionmonkey: (ARM) fix some instruction addressing mode corner cases.

Categories

(Core :: JavaScript Engine, defect)

ARM
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: dougc, Assigned: dougc)

References

Details

Attachments

(1 file, 1 obsolete file)

The ARM Macro-Assembler tries hard to fit an address offset into an efficient instruction sequence but there appear to be some corner cases that are incorrectly encoded.

For example: consider an offset of 0x2C00 for a float32 load.  The logic in MacroAssemblerARM::ma_vdtr calculates a 'bottom' of 0x00, and a 'neg_bottom' of 0x100, and incorrectly encodes a immediate offset of 0x100 into an 8 bit unsigned negative offset instruction field.  

There appear to be similar issues in MacroAssemblerARM::ma_dataTransferN.
This patch just avoids the corner cases, in particular the case in which 'bottom' is zero and thus neg_bottom is out of range for the instruction immediate offset field.
Attachment #775324 - Flags: review?(mrosenberg)
Comment on attachment 775324 [details] [diff] [review]
Avoid some instruction addressing mode corner cases

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

Would it be possible to make this into a single check for bottom == 0?
barring that, there should definitely be a comment describing why bottom == 0 is bad.  It took me a good 15 minutes to figure it out, and I wrote this code...
Attachment #775324 - Flags: review?(mrosenberg)
It does not appear to be possible or useful to hoist the 'bottom != 0' checks because there are useful cases that are ok when 'bottom' is zero.  However the documentation has been improved and some assertions added to help clarify the issue.
Attachment #775324 - Attachment is obsolete: true
Attachment #781707 - Flags: review?(mrosenberg)
Attachment #781707 - Flags: review?(mrosenberg) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/79899f8b526b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.