607225 - nanojit: add MODRM() and SIB() to Nativei386.cpp, plus other clean-ups

Status: RESOLVED FIXED

(Core Graveyard :: Nanojit, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment: patch (against TM 56282:d217b7dccfc6)

This patch cleans up the i386 back-end some more.

- Introduces MODRM() and SIB() functions.  The old MODRM() function was
  renamed MODRMr() to match MODRMm() et al.  Several assertions could be
  removed because MODRM() and SIB() do the relevant assertions now.

- Documents the possible lengths of various instruction fragments.

- Documents the addressing mode encoded by each MODRM() call.  (Diligent
  checking of these would be welcome :)

- Inlines PUSHi32() to its one call-site, removing an excess call to
  count_push() in the process.

- Fixes printing of the SIBIDX(scale) param in several places -- uses %c
  instead of %d.

- Better distinguished functions that write whole instructions vs. functions
  that write instruction fragments.

BTW, I'm planning to add OPCODE()/OPCODE2()/OPCODE3() functions, but I
didn't want to put too much in a single patch.

Comment 1

[Rick Reitmaier]

sorry for delay nick ... reviewing later this eve.

Comment 2

[Rick Reitmaier]

Comment on attachment 485978 [details] [diff] [review]
patch (against TM 56282:d217b7dccfc6)

R+ nice cleanup with a couple of nits that are optional:

(1) I'm rather attached to "Table 2-2. 32-Bit Addressing Forms with the ModR/M Byte" of the intel doc.  This table displays the order of the ModRM bits as 'Mod R/M Reg'.   Although this order differs from bit order it makes decoding and comparison much easier.  

(2) In MODRMr() should we replace the code with a direct call out to MODRM() ?

Comment 3

[Nicholas Nethercote [inactive]]

(In reply to comment #2)
> 
> (1) I'm rather attached to "Table 2-2. 32-Bit Addressing Forms with the ModR/M
> Byte" of the intel doc.  This table displays the order of the ModRM bits as
> 'Mod R/M Reg'.   Although this order differs from bit order it makes decoding
> and comparison much easier.  

I'd prefer to keep the existing order if that's ok!  :)

Comment 4

[Rick Reitmaier]

http://hg.mozilla.org/projects/nanojit-central/rev/5012f8eb917c

http://hg.mozilla.org/tamarin-redux/rev/2c02ae84a295

Comment 5

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/afcdb9b4bb04
http://hg.mozilla.org/mozilla-central/rev/afcdb9b4bb04