Choosing registers for assembly snippets

[Ludovic Henry]

Hi Zixian,

This is where the abstraction can be leaky and the callee (in this
case BarrierSetAssembler::tlab_allocate) needs to know how its called by
callers
(C1_MacroAssembler::allocate_array,
C1_MacroAssembler::allocate_object, TemplateTable::_new
in this case). It might go down to precedents (is it done like that in
similar cases?) and to reviewers.

I'll always prefer a clear contract with clear separation of concerns
between callers and callees, but that needs to be balanced with the
performance overhead of the solution.

I'd be happy to hear of others' points of view on the RISC-V port.

Thanks,
Ludovic

On Wed, Oct 5, 2022 at 12:27 PM Zixian Cai <zixian.cai at anu.edu.au> wrote:

> Hi Ludovic,
>
>
>
> Thanks for the information. Regarding using the registers passed in, is
> there any guarantee in terms of register conflict or we should handle all
> corner cases?
>
> One example I mentioned earlier is, again in TLAB allocate,
> https://github.com/openjdk/jdk/blob/5a9cd33632862aa2249794902d4168a7fe143054/src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp#L155
> .
>
>
>
> The code handles the case where var_size_in_bytes is the same register as
> tmp2. And I can confirm that such conflict indeed happens in practice, and
> if not handled, causes issues, for example, in
> java.util.Arrays.copyOfRange, where the object size register is used after
> allocation to perform the copying.
>
>
>
> Sincerely,
>
> Zixian
>
>
>
> On 5/10/2022, 20:10, "Ludovic Henry" <ludovic at rivosinc.com> wrote:
>
> Hi,
>
> My understanding is it's quite a mixed-bag. There are some places where it
> is assumed which register is used (ex: StubGenerator::generate_zero_blocks
> assumes x28 and x29 are used in MacroAssembler::zero_words) while other
> places pass the registers to be used. It's surprising that the TLAB
> allocation passes the registers to be used (tmp1 and tmp2) but there are
> cases where tmp1 is noreg (like templateTable_riscv.cpp [1]).
>
>
>
> In that specific case of TLAB allocation, it seems to me the right
> approach is to make sure `tmp1` is always valid (AFAIU, the only required
> change is to pass `t0` in [1]), and use `tmp1` in place of `t0` in
> BarrierSetAssembler::tlab_allocate. It would also require asserting that
> both tmp1 and tmp2 are not noreg.
>
>
>
> Finally, on always using t0 and t1 as temporary registers, it's the case
> most of the time, but there are cases where you can't
> (StubGenerator::generate_zero_blocks and MacroAssembler::zero_words for
> example).
>
>
>
> So IMHO the best practice is to pass the registers you can use. The
> exception is when you can't (like the StubGenerator which generates the
> code at startup); then it's a tradeoff between passing the parameters as
> normal arguments (c_rarg0, c_rarg1, etc.) or make an assumption or where
> the value is currently stored (x28. x29 in MacroAssembler::zero_words case
> for example).
>
>
>
> Cheers,
>
> Ludovic
>
>
>
> [1]
> https://github.com/openjdk/jdk/blob/953ce8da2c7ddd60b09a18c7875616a2477e5ba5/src/hotspot/cpu/riscv/templateTable_riscv.cpp#L3406
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/riscv-port-dev/attachments/20221005/3569a49f/attachment.htm>