RFR: 8318217: RISC-V: C2 VectorizedHashCode [v2]

[Yuri Gaevsky]

On Wed, 29 Nov 2023 17:50:00 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi @Hamlin-Li,
>> 
>> My apologies for some delay with an answer.
>> 
>> Without specifying the "concrete" registers for _ary/cnt/result_, as e.g. as follows:
>> (1) [ iRegP/iRegI ]
>> 
>> instruct arrays_hashcode(iRegP ary, iRegI cnt, iRegI result, immI basic_type,
>>                          iRegLNoSp tmp1, iRegINoSp tmp2,
>>                          iRegINoSp tmp3, iRegLNoSp tmp4, rFlagsReg cr)
>> 
>> or
>> (2) [ iRegPNoSp/iRegINoSp ]
>> 
>> instruct arrays_hashcode(iRegPNoSp ary, iRegINoSp cnt, iRegINoSp result, immI basic_type,
>>                          iRegLNoSp tmp1, iRegINoSp tmp2,
>>                          iRegINoSp tmp3, iRegLNoSp tmp4, rFlagsReg cr)
>> 
>> it's impossible to use '_USE_KILL_' hint in '_effect_' directive because AD compilation fails:
>> 
>> Building target 'images' in configuration 'linux-riscv64-server-release'
>> /jdk/src/hotspot/cpu/riscv/gc/z/z_riscv.ad(10306) Syntax Error: :In arrays_hashcode only bound registers can be killed: iRegP ary
>> 
>> or
>> 
>> Building target 'images' in configuration 'linux-riscv64-server-release'
>> /jdk/src/hotspot/cpu/riscv/gc/z/z_riscv.ad(10306) Syntax Error: :In arrays_hashcode only bound registers can be killed: iRegPNoSp ary
>> 
>> 
>> IMHO, the usage of '_USE_KILL_' for _ary_/_cnt_ looks reasonable since
>> we actually do use/modify them both in the assembler body, and avoiding any
>> hint or usage other hint ('_USE_'?) may be wrong here even if that does
>> not cause a failure in pre-integration tests. It is interesting that many
>> intrinsics' definitions in AD files similarly use "concrete" registers in
>> X86/RISC-V archs, possibly due to the mentioned "_only bound registers can be
>> killed_" ADLC requirement.
>> 
>> I'd like to mention that applying '_USE_KILL_' to 'result' causes the error
>> in runtime:
>> 
>> ---------------  T H R E A D  ---------------
>> 
>> Current thread (0x0000003fc0189130):  JavaThread "C2 CompilerThread0" daemon [_thread_in_native, id=54524, stack(0x0000003f7bc00000,0x0000003f7be00000) (2048K)]
>> 
>> 
>> Current CompileTask:
>> C2:828  117       4       java.lang.String::hashCode (60 bytes)
>> 
>> Stack: [0x0000003f7bc00000,0x0000003f7be00000],  sp=0x0000003f7bdfbba0,  free space=2030k
>> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
>> V  [libjvm.so+0x5e1ad2]  Node_Backward_Iterator::next()+0x106
>> V  [libjvm.so+0x5e4040]  PhaseCFG::global_code_motion()+0x242
>> V  [libjvm.so+0x5e5266]  PhaseCFG::do_global_code_motion()+0x38
>> V  [libjvm.so+0x455142]  Compile::Cod...
>
> Thanks for looking into the details.
> Based on the information we got till now, yes, we need the concrete registers, otherwise, we need to pass in more tmp register, seems former one is better, let's go with it.

Thanks @Hamlin-Li, the patch has been updated to use concrete registers.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16629#discussion_r1409766224