RFR: 8320397: RISC-V: Avoid passing t0 as temp register to MacroAssembler:: cmpxchg_obj_header/cmpxchgptr [v2]

[Robbin Ehn]

On Thu, 14 Dec 2023 08:45:34 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> I tested removing all "KILL cr" from all riscv ad files. 
>> CORRECTION:
>> Running compiler tests c2/codegen: something blow up :) 
>> 
>> My theory was, as we never consume or produce values in 'cr' we can see cr as always having correct values, which is nothing. 
>> 
>> It seem like we need to KILL cr, not to confuse C2 to much :)
>
> Hi @robehn, I am still a bit confused. Are you suggesting we keep the `KILL cr` in `effect`? My first comment is suggesting we remove that. Of couse, it will still work in functionality if we simply keep it there. But I am a bit worried that this may affect performance in some way as our CR could contain a live value in nodes as I mentioned in my previous comment. A `KILL cr` in `effect` as in this case may pose some unnecessary constraint on C2 code motion. That's just my concern.

This is not related to this bug (yes, we can remove KILL cr):

Ah, this is very problematic, as t1 is used masm, which means calling masm may destroy cr.

For exampel C2 may call:
`MacroAssembler::zero_words(Register ptr, Register cnt)` as it do not clobber t1 but if we would call:
`MacroAssembler::zero_words(Register base, uint64_t cnt) ` which do clobber t1 suddenly we need to add KILL cr.

Which means if someone just do a small change like:

- masm->zero_words(reg1, reg2);
+ masm->zero_words(reg1, 64);


We suddenly can get very subtle bugs.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16880#discussion_r1426475012