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

Robbin Ehn rehn at openjdk.org
Thu Dec 14 10:29:37 UTC 2023


On Thu, 14 Dec 2023 10:06:39 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> This is not related to this enhancement (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.
>
> Yeah, I agree it is error-prone. But it is the same case for ther other platforms which have a real CR register, isn't it? They should have the same issue. I guess it doesn't deserve it if we keep `t1` the dedicated CR register, that is not touched else where by other masm assembers.

I think we can pretty easy add a scoped object, similar to uncompressed region, where we can white-list registers.

So the ad file would add for example to fast_lock:


{
  AllowAdditionalRegister(t1); // As we kill cr fastlock may use t1/(cr) in addition to t0 and passed in regs.
  ___ fastlock(...);
}

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

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


More information about the hotspot-dev mailing list