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

[Fei Yang]

On Thu, 14 Dec 2023 07:41:51 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> 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)

> 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?

The C2 JIT does expect something in a CR register. Check the nodes in file riscv.ad like `cmpFastLock`, `cmpFastUnlock` and `partialSubtypeCheckVsZero` [1]. That's way we emulated the CR register on riscv using the `t1` general-purpose register. I don't see another way without changing the C2's assumption about CR. 

[1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/riscv.ad#L10434

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

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