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

[Hamlin Li]

On Wed, 29 Nov 2023 12:05:23 GMT, Yuri Gaevsky <duke at openjdk.org> wrote:

>> I think the reason might be: with specific register, you can add effect as `USE_KILL ary, USE_KILL cnt`, but without specific register, currently you have to way to do so.
>> But, in current patch, it does modify the ary and cnt in the intrinsic, so I wonder if the current (lastest) patch is safe enough in all situation.
>> 
>> It maybe be helpful to add 2 new register when matching the instrinsic in ad file, and I guess the register allocator will merge different use of temp register together?
>> But I still think it's not necessary to specify the register when matching arrays_hashcode in ad file.
>
> 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::Code_Gen()+0x198
> V  [libjvm.so+0x458168]  Compile::Compile(ciEnv*, ciMethod*, int, Options, DirectiveSet*)+0xce...

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.

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

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