RFR: 8225429: Regression in disassembly quality

[Tom Rodriguez]

I didn't see the original review for the disassembly change but even in 
product you used to get most of this information.  Assembler comment 
capturing is a non-product feature unfortunately, so those were never 
available.  Was it intentional to turn all this output off in product? 
I'd rather see it restored even in product myself.

I had made some fixes to the alignment logic that I was planning to send 
out for review.  The whole calculate_alignment logic seems broken to me. 
  It's only called when it matches the addr event, but there's already 
an addr check above this call so it's unreachable.  My changes are at 
http://cr.openjdk.java.net/~never/printnm/webrev if you would want to 
incorporate parts of it.  I also fixed a problem where the block comment 
prefix lost a single space, which doesn't show up properly in the 
webrev.  I changed the calculate_alignment so that there's at most a 8 
chars between the disassembly and the comment.

tom

Andrew Haley wrote on 6/6/19 8:19 AM:
> In a debug build, if you printed a disassembled method, you used to
> get a lot of information about a method. Now, you just get raw
> disassembly. This patch restores the status quo ante,
> 
> http://cr.openjdk.java.net/~aph/8225429-1/
> 
> There are other bugs in the new disassembly output: in particular
> options processing seems to be very broken. However, this patch is
> enough to get things going again, so at least we can get some work
> done.
> 
> Before:
> 
>    # {method} {0x00007f2f64833660} 'decodeModuleName' '(ILjava/lang/String;)Ljava/lang/String;' in 'jdk/internal/module/ModuleInfo$ConstantPool'
>    # this: rsi:rsi = 'jdk/internal/module/ModuleInfo$ConstantPool'
>    # parm0: rdx = int
>    # parm1: rcx:rcx = 'java/lang/String'
>    # [sp+0x40] (sp of caller)
>   ;; N542: # B1 <- BLOCK HEAD IS JUNK Freq: 1
> 
>    0x00007f2fb1168440: mov r10d,DWORD PTR [rsi+0x8]
>    0x00007f2fb1168444: movabs r12,0x800000000
>    0x00007f2fb116844e: add r10,r12
>    0x00007f2fb1168451: xor r12,r12
>    0x00007f2fb1168454: cmp rax,r10
>    0x00007f2fb1168457: jne 0x00007f2fb104d6a0 ; {runtime_call ic_miss_stub}
>    0x00007f2fb116845d: nop
>    0x00007f2fb116845e: nop
>    0x00007f2fb116845f: nop
> [Verified Entry Point]
>   ;; B1: # B54 B2 <- BLOCK HEAD IS JUNK Freq: 1
> 
>    0x00007f2fb1168460: mov DWORD PTR [rsp-0x16000],eax
>    0x00007f2fb1168467: push rbp
>    0x00007f2fb1168468: sub rsp,0x30 ;*synchronization entry
>                                                  ; - jdk.internal.module.ModuleInfo$ConstantPool::decodeModuleName at -1 (line 877)
> 
>    0x00007f2fb116846c: mov r10,rsi
>    0x00007f2fb116846f: mov r9,rcx
>    0x00007f2fb1168472: mov edi,DWORD PTR [rcx+0xc] ;*getfield value {reexecute=0 rethrow=0 return_oop=0}
>                                                  ; - java.lang.String::length at 1 (line 658)
>                                                  ; - jdk.internal.module.ModuleInfo$ConstantPool::decodeModuleName at 1 (line 877)
>                                                  ; implicit exception: dispatches to 0x00007f2fb116872a
>   ;; B2: # B55 B3 <- B1 Freq: 0.999999
> 
> Now you only get this, even in a debug build:
> 
>    0x00007ff1c5176c20: mov r10d,DWORD PTR [rsi+0x8]
>    0x00007ff1c5176c24: movabs r12,0x800000000
>    0x00007ff1c5176c2e: add r10,r12
>    0x00007ff1c5176c31: xor r12,r12
>    0x00007ff1c5176c34: cmp rax,r10
>    0x00007ff1c5176c37: jne 0x00007ff1c5113da0
>    0x00007ff1c5176c3d: nop
>    0x00007ff1c5176c3e: nop
>    0x00007ff1c5176c3f: nop
>    0x00007ff1c5176c40: mov DWORD PTR [rsp-0x16000],eax
>    0x00007ff1c5176c47: push rbp
>    0x00007ff1c5176c48: sub rsp,0x30
>    0x00007ff1c5176c4c: mov r11,rsi
>    0x00007ff1c5176c4f: mov r9,rcx
>    0x00007ff1c5176c52: mov r10d,DWORD PTR [rcx+0xc]
> 
> There are no comments at all, just raw disassembly.
> 
> This patch restores the status quo ante:
> 
>    # {method} {0x00007f9f53c2e3c0} 'decodeModuleName' '(ILjava/lang/String;)Ljava/lang/String;' in 'jdk/internal/module/ModuleInfo$ConstantPool'
>    # this:     rsi:rsi   = 'jdk/internal/module/ModuleInfo$ConstantPool'
>    # parm0:    rdx       = int
>    # parm1:    rcx:rcx   = 'java/lang/String'
>    #           [sp+0x40]  (sp of caller)
>   ;; N551: #     out( B1 ) <- BLOCK HEAD IS JUNK  Freq: 1
>    0x00007f9fb4d64620:   mov    r10d,DWORD PTR [rsi+0x8]
>    0x00007f9fb4d64624:   movabs r12,0x800000000
>    0x00007f9fb4d6462e:   add    r10,r12
>    0x00007f9fb4d64631:   xor    r12,r12
>    0x00007f9fb4d64634:   cmp    rax,r10
>    0x00007f9fb4d64637:   jne    0x00007f9fb4cfeda0           ;   {runtime_call ic_miss_stub}
>    0x00007f9fb4d6463d:   nop
>    0x00007f9fb4d6463e:   nop
>    0x00007f9fb4d6463f:   nop
> [Verified Entry Point]
>   ;; B1: #       out( B55 B2 ) <- BLOCK HEAD IS JUNK  Freq: 1
>    0x00007f9fb4d64640:   mov    DWORD PTR [rsp-0x16000],eax
>    0x00007f9fb4d64647:   push   rbp
>    0x00007f9fb4d64648:   sub    rsp,0x30                     ;*synchronization entry
>                                                              ; - jdk.internal.module.ModuleInfo$ConstantPool::decodeModuleName at -1 (line 876)
>    0x00007f9fb4d6464c:   mov    r11,rsi
>    0x00007f9fb4d6464f:   mov    r9,rcx
>    0x00007f9fb4d64652:   mov    r10d,DWORD PTR [rcx+0xc]     ;*getfield value {reexecute=0 rethrow=0 return_oop=0}
>                                                              ; - java.lang.String::length at 1 (line 673)
>                                                              ; - jdk.internal.module.ModuleInfo$ConstantPool::decodeModuleName at 1 (line 876)
>                                                              ; implicit exception: dispatches to 0x00007f9fb4d648fe
>   ;; B2: #       out( B56 B3 ) <- in( B1 )  Freq: 0.999999
>