What Do You Think? JDK-8213084: Rework and enhance Print[Opto]Assembly output

[Andrew Haley]

Hi,

On 11/29/18 4:02 PM, Schmidt, Lutz wrote:
>
> @Andrew On your specific comments:

>  - With "vertical whitespace" you mean vertical (columnar)
> alignment, I assume. No need to be afraid, vertical alignment is
> very important to me as well. X86 was rather well aligned from the
> beginning, other platforms not so.

No, "vertical whitespace" means blank lines. They are important.

>  - Empty (blank) lines: That's a matter of personal taste. When
> looking at a larger piece of code, I like the more condensed
> format. Furthermore, you can fit more instructions on one
> page/screen.

The advantage of whitespace isn't entirely personal: it can be shown
by experiment that it aids comprehension, as referred to by Dmitry
Fadeyev among others.

The key to readability is about presenting everything in the briefest
obvious and unambiguous format. That's why notations such as method at 95
to indicate a bytecode index are good: sure, you have to learn them
but once you do they're easy to see.

Compare and contrast:

 ;; B1: #       B87 B2 <- BLOCK HEAD IS JUNK   Freq: 1

  0x00000fff6d396710: mflr    r20
  0x00000fff6d396714: addis   r11,r1,-2
  0x00000fff6d396718: std     r0,0(r11)
  0x00000fff6d39671c: mr      r21,r1
  0x00000fff6d396720: stdu    r1,-96(r1)
  0x00000fff6d396724: std     r20,16(r21)                   ;*synchronization entry
                                                            ; - spec.benchmarks._201_compress.Compressor::compress at -1



 ;; block B1: # (out B114 B2 <- BLOCK HEAD IS JUNK  ) Freq: 1
  0x00000fff8bdf1d10 (+0x0010): 7e88 02a6 |             mflr    r20
  0x00000fff8bdf1d14 (+0x0014): 3d61 ffff |             addis   r11,r1,-1
  0x00000fff8bdf1d18 (+0x0018): f80b 0000 |             std     r0,0(r11)
  0x00000fff8bdf1d1c (+0x001c): 7c35 0b78 |             mr      r21,r1
  0x00000fff8bdf1d20 (+0x0020): f821 ff91 |             stdu    r1,-112(r1)
  0x00000fff8bdf1d24 (+0x0024): fa95 0010 |             std     r20,16(r21)             ;*synchronization entry
                                                                                        ; - spec.benchmarks._201_compress.Compressor::compress at -1

Obviously, the first major difference is the width. Yours is very
wide, at 145 columns. But, weirdly, it is very miserly with vertical
space. So you're happy to use horizontal space, presumably to increase
readability, but you want to save vertical space at all costs.

>From left to right:

Why "block B1" rather than "B1"? It's a block; we know that already.

The (0x1234) offset helps a little bit, but nothing in the disassembly
refers to it, so it's not doing much.

The hex format for instructions might be useful in some limited
contexts such as redundant encodings, but you probably don't need to
know which of the seven kinds of NOP has been emitted.

The "|             " does nothing useful at all.

Getting more consistent alignment is useful, I won't deny that. But
forcing users to make their terminal window even wider when looking at
disassembly isn't nice.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671