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

[Schmidt, Lutz]

Hi Andrew, Tobias,

thank you for your input! That's exactly what I hoped for: everything from awful to awesome. OK, the middle part is still missing. Maybe there are others expressing their opinion in the coming days. Such a change will never make everybody completely happy. I will try to adapt to cause only acceptable pain for everyone. 

@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.
 - 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.
 - Block header lines: I like the in-block and out-block grouping by parentheses (or any other character). No emotional connection with everything else. Suggestion:
     ;; B3: #       (B5 B6) <- (B1 B2 B3 B4)  Freq: 0.999979

That's it for now. Further comments welcome, of course.

Thanks,
Lutz


On 29.11.18, 11:54, "Andrew Haley" <aph at redhat.com> wrote:

    On 11/28/18 4:16 PM, Schmidt, Lutz wrote:
    
    > Looking forward to your comments, whatever they may be. And, please, cross-post to other lists if you see a requirement.
    
    Current:
    
    ----------------------------------------------------------------------------------------------------
    ;; B3: #       B112 B4 <- B2  Freq: 0.999979
    
      0x00007f2cdfd26d08: mov    (%rsp),%r10
      0x00007f2cdfd26d0c: mov    0x4c(%r10),%r9d                ;*getfield htab {reexecute=0 rethrow=0 return_oop=0}
                                                                ; - spec.benchmarks._201_compress.Compressor::compress at 15
    
      0x00007f2cdfd26d10: mov    0xc(%r9),%r11d                 ;*getfield size {reexecute=0 rethrow=0 return_oop=0}
                                                                ; - spec.benchmarks._201_compress.Compressor::compress at 22
                                                                ; implicit exception: dispatches to 0x00007f2cdfd27383
    ----------------------------------------------------------------------------------------------------
    Yours:
    
    ----------------------------------------------------------------------------------------------------
    ;; block B3: # (out B115 B4 <- (in B2 )) Freq: 0.999979
      0x00007fe1b77ff364 (+0x0044): mov    0x4c(%rbp),%r10d         ;*getfield htab {reexecute=0 rethrow=0 return_oop=0}
                                                                    ; - spec.benchmarks._201_compress.Compressor::compress at 15
      0x00007fe1b77ff368 (+0x0048): mov    0xc(%r10),%r8d           ;*getfield size {reexecute=0 rethrow=0 return_oop=0}
                                                                    ; - spec.benchmarks._201_compress.Compressor::compress at 22
                                                                    ; implicit exception: dispatches to 0x00007fe1b77ffdfa
    ----------------------------------------------------------------------------------------------------
    
    
    Current is better than Yours, IMO. Vertical whitespace is good, please don't
    lose it.
    
    Your constant pool output is extremely verbose, and this is not an improvement.
    There must be some compromise.
    
    Whenever you add more detail, there is always the risk of hiding what is
    essential. Why this:
    
    ;; block B3: # (out B115 B4 <- (in B2 )) Freq: 0.999979
    
    rather than
    
    ;; B3: #       B112 B4 <- B2  Freq: 0.999979
    
    ?
    
    -- 
    Andrew Haley
    Java Platform Lead Engineer
    Red Hat UK Ltd. <https://www.redhat.com>
    EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671