RFR: 8326385: [aarch64] C2: lightweight locking nodes kill the box register without specifying this effect

[Axel Boldt-Christmas]

On Mon, 11 Mar 2024 23:15:50 GMT, Dean Long <dlong at openjdk.org> wrote:

>> This changeset introduces a third `TEMP` register for the intermediate computations in the `cmpFastLockLightweight` and `cmpFastUnlockLightweight` aarch64 ADL instructions, instead of using the legacy `box` register. This prevents potential overwrites, and consequent erroneous uses, of `box`.
>> 
>> Introducing a new `TEMP` seems conceptually simpler (and not necessarily worse from a performance perspective) than pre-assigning `box` an arbitrary register and marking it as `USE_KILL`, an alternative also suggested in the [JBS issue description](https://bugs.openjdk.org/browse/JDK-8326385). Compared to mainline, the changeset does not lead to any statistically significant regression in a set of locking-intensive benchmarks from DaCapo, Renaissance, SPECjvm2008, and SPECjbb2015.
>> 
>> #### Testing
>> 
>>  - tier1-7 (linux-aarch64 and macosx-aarch64) with `-XX:LockingMode=2`.
>
> src/hotspot/cpu/aarch64/aarch64.ad line 16026:
> 
>> 16024:   predicate(LockingMode == LM_LIGHTWEIGHT);
>> 16025:   match(Set cr (FastLock object box));
>> 16026:   effect(TEMP tmp, TEMP tmp2, TEMP tmp3);
> 
> Why not use `box` as the temp instead of intruducing a separate temp?
> Suggestion:
> 
>   effect(TEMP tmp, TEMP tmp2, TEMP box);

Making an input TEMP will crash inside C2 when doing register allocation. 

assert(opcnt < numopnds) failed: Accessing non-existent operand

V  [libjvm.dylib+0xcc650c]  MachNode::in_RegMask(unsigned int) const+0x1f0
V  [libjvm.dylib+0x3d136c]  PhaseChaitin::gather_lrg_masks(bool)+0x1130
V  [libjvm.dylib+0x3cef9c]  PhaseChaitin::Register_Allocate()+0x150
V  [libjvm.dylib+0x4c3860]  Compile::Code_Gen()+0x1f4
V  [libjvm.dylib+0x4c17a4]  Compile::Compile(ciEnv*, ciMethod*, int, Options, DirectiveSet*)+0x1388
V  [libjvm.dylib+0x38abf0]  C2Compiler::compile_method(ciEnv*, ciMethod*, int, bool, DirectiveSet*)+0x1e0
V  [libjvm.dylib+0x4df028]  CompileBroker::invoke_compiler_on_method(CompileTask*)+0x854
V  [libjvm.dylib+0x4de46c]  CompileBroker::compiler_thread_loop()+0x348
V  [libjvm.dylib+0x8c10e4]  JavaThread::thread_main_inner()+0x1dc
V  [libjvm.dylib+0x117f7f8]  Thread::call_run()+0xf4
V  [libjvm.dylib+0xe53724]  thread_native_entry(Thread*)+0x138
C  [libsystem_pthread.dylib+0x7034]  _pthread_start+0x88


Maybe this can be resolved and support for TEMP input registers can be added.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18183#discussion_r1520969175