560712 - nanojit: give immediates names when printing LIR

Status: RESOLVED FIXED

(Core Graveyard :: Nanojit, defect)

Description

[Nicholas Nethercote [inactive]]

Bug 518267 changes debug printing so that live immediates are always printed.  Ie. this:

  x = add y, 0

Becomes this:

  0 = imml 0
  x = add y, 0

I want to further change immediates them to be more like everything else:

  imml1 = imml 0
  x = add y, imml1

It will simplify the code, and make it more obvious how immediates are used, particularly if immediate operands are added (bug 559949).  Ie. the difference between the previous example and the following will be clear:

  x = addi y, 0

ie. '0' is obviously an inlined immediate, not a reference to a prior instruction.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: patch

Makes the change described in comment 0.  formatImm() and formatImmq() could be deleted as a result.  I fixed the range tests which were like this:

   -10000 < c || c < 10000

to this:

   -10000 < c && c < 10000

Yikes!  I also changed 'i' to the more descriptive 'ins' in formatIns().

Nb: I don't plan landing this until more opcode renaming has been completed.

Comment 2

[Edwin Smith]

Comment on attachment 440421 [details] [diff] [review]
patch

This is a sort of contingent R+

I find it extremely useful to see the value of the immediate printed at the point of reference, but I'm not very choosy about how that value is printed, and not opposed to printing the name and value.  some possibilities:

  imml1 = imml 0
  x = add y, imml1=0   // syntax is name=value

  imml1 = imml 0
  x = add y, imml1/*0*/  // syntax is name/*value*/

  imml1 = imml 0
  x = add y, imml1(0)    // syntax is name(value)

Comment 3

[Steven Johnson]

(In reply to comment #2)
> I find it extremely useful to see the value of the immediate printed at the
> point of reference

+1

how about

  imml1 = imml 0
  x = add y, imml1 // = 0  // syntax is name // = value

Comment 4

[Edwin Smith]

I didn't suggest that syntax, because references to immediates aren't always the last thing on a line, for example:

   ptr = imml 0x12345678
   calli1 = calli #somefunc (... ptr ...)

might still be readable as:

   ptr = imml 0x12345678
   calli1 = calli #somefunc (... ptr=0x12345678 ...)

Comment 5

[Nicholas Nethercote [inactive]]

Yeah, fair enough.  I'll fiddle with the syntax and adjust the patch.

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: patch v2

This patch:
  
- Prints both the name and value for immediate references.  Examples:
  
    12: immi3 = immi 0
    13: eqi1 = eqi ldi2, immi3/*0*/
    14: xf2: xf eqi1 -> pc=0x8540251 imacpc=(nil) sp+0 rp+0 (GuardID=001)
    15: immd3 = immd 3.3
    16: std.s sp[0] = immd3/*3.3*/
    17: std.s sp[-8] = immd3/*3.3*/
  
  I went with "/*n*/" form because it is enclosed and so won't cause problems
  if anything is printed afterwards, and I thought it less likely to be
  misinterpreted than "=n" or "(n)", both of which have other meanings already
  in the output (assignment and function call, respectively).
  
  This required adding a boolean param 'showImmValue' (which defaults to
  true) to formatRef(), to avoid printing "imml1/*1*/ = 1" when immediates
  are defined.

- Adds formatImmD() for consistency.
    
- formatImm*() now return the printed string.

Comment 7

[Nicholas Nethercote [inactive]]

NJ: http://hg.mozilla.org/projects/nanojit-central/rev/535221682568
TM: http://hg.mozilla.org/tracemonkey/rev/3a5a84f83e75

Comment 8

[Edwin Smith]

TR: http://hg.mozilla.org/tamarin-redux/rev/3d0a14f5c558

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/3a5a84f83e75