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

[Ioi Lam]

Hi Tobias, thanks for the review. I will fix the typos and push.

- Ioi

On 5/14/19 2:58 AM, Tobias Hartmann wrote:
> 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
>>
>>