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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Douglas Crosher [:dougc]]

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.

Comment 1

[Douglas Crosher [:dougc]]

Attachment: Avoid some instruction addressing mode corner cases

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.

Comment 2

[Marty Rosenberg [:mjrosenb]]

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...

Comment 3

[Douglas Crosher [:dougc]]

Attachment: Revised patch attempting to address review comments.

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.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/79899f8b526b

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/79899f8b526b