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 07:44:45 UTC 2023
On Thu, 14 Dec 2023 02:52:34 GMT, Fei Yang <fyang at openjdk.org> wrote:
>> Gui Cao has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Add tmpReg to replace the t1 register
>
> src/hotspot/cpu/riscv/gc/x/x_riscv.ad line 78:
>
>> 76: match(Set res (WeakCompareAndSwapP mem (Binary oldval newval)));
>> 77: predicate(UseZGC && !ZGenerational && !needs_acquiring_load_reserved(n) && n->as_LoadStore()->barrier_data() == XLoadBarrierStrong);
>> 78: effect(KILL cr, TEMP_DEF res, TEMP tmp);
>
> You might want to remove `KILL cr` from `effect` at the same time if `cr` (aka `t1`) is not used anymore after this change.
I don't *think* that matters, as masm may use t1 so this 'may' kill cr.
But since cr never contains any actual information and is never read (as cr), it's unclear to me, if we ever can get unwanted side-effect if either missing a "KILL cr" or have one to many, you know?
As we have fused cmp+branch I would think the graph would be better of believing we always have a live CR.
We don't need to re-do the cmp to get back the killed cr, we can pretend it is still alive, since the fused branch will act as it is alive, no?
Also I have patch which cleans-up the registers in riscv.ad, I'll try to get it out as RFC in form of a PR. (i.e. my suggestions)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16880#discussion_r1426349293
More information about the hotspot-dev
mailing list