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

[Schmidt, Lutz]

Hi Andrew, 

thanks for your extensive answer! I really appreciate it. 

There is a tradeoff to be made between optical structuring by whitespace
(be it horizontal or vertical) and compactness. I personally have a
noticeable bias towards compactness and therefore agree to your 
statement about readability. 

With the above said, why is there so much horizontal whitespace?
 - For some platforms, the hsdis library inserts a tab between mnemonic
   and operands to align the operands. That only works if the mnemonic
   is tab-aligned as well. That adds up to seven spaces.
 - On PPC, an additional eight character field is provided for 
   supplemental information (not yet contributed to OpenJDK).
   Refers to your comment:
     The "|             " does nothing useful at all.
 - Position of the code comments is such that there is room for all 
   but the very longest operand lists. Intention: preserve vertical
   alignment.

Comments on other objections:
 - "block B1" should be "B1", agreed.
 - The offset (hex or decimal) allows me to judge distances more easily.
 - "instruction in hex" is configurable anyway. Not everybody likes it.

I would suggest to suspend the discussion here until other opinions have
been submitted. Not because I want to suppress the discussion, by far not.
I assume you have other, at least as important topics to work on.

Thank you for your time!
Lutz


On 30.11.18, 11:03, "Andrew Haley" <aph at redhat.com> wrote:

    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