RFR: 8320397: RISC-V: Avoid passing t0 as temp register to MacroAssembler:: cmpxchg_obj_header/cmpxchgptr [v2]
Fei Yang
fyang at openjdk.org
Thu Dec 14 10:09:38 UTC 2023
On Thu, 14 Dec 2023 09:41:32 GMT, Robbin Ehn <rehn at openjdk.org> wrote:
>> 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 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16880#discussion_r1426505395
More information about the hotspot-dev
mailing list