RFR 8223729 [lworld] Initial C1 implementation for -XX:ACmpOnValues=3

[Tobias Hartmann]

Hi Ioi,

this looks good to me!

Small things:
- It should be "substitutability" not "substituability".
- In c1_LIRGenerator.cpp: "We do this only if we are not sure that the operands _are_ value objects"

Thanks,
Tobias


On 11.05.19 00:32, Ioi Lam wrote:
> https://bugs.openjdk.java.net/browse/JDK-8223729
> http://cr.openjdk.java.net/~iklam/valhalla/8223729-initial-c1-acmp-on-values-3.v01/
> 
> This is a rough implementation that passes all the tests in TestNewAcmp, but more optimizations can
> be done (see FIXME in the code).
> 
> If both operands are known to be inlined instances, the generated code is:
> 
>        76: cmp    %rdx,%rsi             ; x == y ?
>        79: je      L_2
> 
>        85: cmp    $0x0,%rsi             ; null check for x
>        89: je      L_4
>        95: cmp    $0x0,%rdx             ; null check for y
>        99: je      L_4
>       105: jmpq    L_5
> 
>   L_4 110: mov    $0x0,%edi             ; not equal
>       115: jmpq    L_7
> 
> 
>   L_5 120: mov    %rsi,%rdi             ; shuffle parameters for isSubstitutable(x, y)
>       123: mov    %rsi,0x28(%rsp)       ; why is this code here?
>       128: mov    %rdi,%rsi
>       131: mov    %rdx,%rdi
>       134: mov    %rdx,0x20(%rsp)
>       139: mov    %rdi,%rdx
> 
>       142: nop
>       143: callq  0x00007f9ca0865c20    ; ValueBootstrapMethods::isSubstitutable()
> 
>       148: cmp    $0x1,%eax
>       151: mov    $0x0,%edi
>       156: jne     L_7
> 
>   L_2 162: mov    $0x1,%edi
> 
>   L_7 ....
> 
> 
> Notes:
> 
> (1) I decided to do the explicit null check, as it may be common to do something like
> 
>     Object x = .., y = a_static_field_thats_null;
>     if (x == y) {...}
> 
>     and I don't want to trigger a signal for every acmp in this situation.
> 
> (2) The explicit null check can be removed if both operands are not nullable
>     (or with profiling).
> 
> (3) If x and y are of type Object, etc, there are more checks (lines 41 ~ 89 below).
>     As I mentioned in the FIXME, lines 41~75 can be simplified down to about
>     5 instructions on x64.
> 
>        12: cmp    %rdx,%rsi
>        15: je      L_1
>        21: cmp    $0x0,%rsi
>        25: je      L_5
>        31: cmp    $0x0,%rdx
>        35: je      L_5
> 
>        // check if x and y are inline types
>        41: mov    $0x405,%rdi
>        51: mov    (%rsi),%rbx
>        54: and    %rdi,%rbx
>        57: cmp    %rdi,%rbx
>        60: jne     L_5
>        66: mov    (%rdx),%rbx
>        69: and    %rdi,%rbx
>        72: cmp    %rdi,%rbx
>        75: jne     L_5
> 
>        // check if x and y of the same type
>        81: mov    0x8(%rsi),%edi
>        84: mov    0x8(%rdx),%ebx
>        87: cmp    %ebx,%edi
>        89: je      L_6
> 
>   L_5  95: mov    $0x0,%edi
>       100: jmpq    L_8
> 
> (4) I had to add "-XX:CompileCommand=exclude,java.lang.ClassValue::*" to TestNewAcmp.java for
>     C1, or else ValueBootstrapMethods::isSubstitutable() works incorrectly. I'll investigate that
>     in a follow-on bug.
> 
> Thanks
> - Ioi
> 
>