<div dir="ltr">Hi Zixian,<div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>I'd be happy to hear of others' points of view on the RISC-V port.</div><div><br></div><div>Thanks,</div><div>Ludovic</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Oct 5, 2022 at 12:27 PM Zixian Cai <<a href="mailto:zixian.cai@anu.edu.au">zixian.cai@anu.edu.au</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="msg2555085198445213621">
<div lang="EN-AU" style="overflow-wrap: break-word;">
<div class="m_2555085198445213621WordSection1">
<p class="MsoNormal">Hi Ludovic,<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">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?<u></u><u></u></p>
<p class="MsoNormal">One example I mentioned earlier is, again in TLAB allocate, <a href="https://github.com/openjdk/jdk/blob/5a9cd33632862aa2249794902d4168a7fe143054/src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp#L155" target="_blank">
https://github.com/openjdk/jdk/blob/5a9cd33632862aa2249794902d4168a7fe143054/src/hotspot/cpu/riscv/gc/shared/barrierSetAssembler_riscv.cpp#L155</a>.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">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. <u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal"><span lang="EN-US">Sincerely,<u></u><u></u></span></p>
<p class="MsoNormal"><span lang="EN-US">Zixian</span><u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal" style="margin-bottom:12pt">On 5/10/2022, 20:10, "Ludovic Henry" <<a href="mailto:ludovic@rivosinc.com" target="_blank">ludovic@rivosinc.com</a>> wrote:<u></u><u></u></p>
<div>
<p class="MsoNormal">Hi,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">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]).<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">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.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">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).<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">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).<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Cheers,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Ludovic<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">[1] <a href="https://github.com/openjdk/jdk/blob/953ce8da2c7ddd60b09a18c7875616a2477e5ba5/src/hotspot/cpu/riscv/templateTable_riscv.cpp#L3406" target="_blank">
https://github.com/openjdk/jdk/blob/953ce8da2c7ddd60b09a18c7875616a2477e5ba5/src/hotspot/cpu/riscv/templateTable_riscv.cpp#L3406</a>
<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div>
</div></blockquote></div>