[lworld] RFR: 8366717: [lworld] Cleanup defensive fixing of JDK-8365996 [v2]

[Marc Chevalier]

On Fri, 19 Dec 2025 08:30:37 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

>> src/hotspot/cpu/aarch64/gc/shared/cardTableBarrierSetAssembler_aarch64.cpp line 117:
>> 
>>> 115:     if (!precise || (dst.index() == noreg && dst.offset() == 0)) {
>>> 116:       if (tmp3 != noreg) {
>>> 117:         // When tmp3 is given, we cannot corrupt the dst.base() register (from MacroAssembler::pack_inline_helper or do_oop_store)
>> 
>>> it doesn't look like an accident. If we remove the if-branch, tests are totally on fire. Let's keep it.
>> 
>> It's only the `__ mov(tmp3, dst.base());` that could potentially be removed, right? Not the entire branch. If the `mov` is still needed, should it be guarded by `InlineTypePassFieldsAsArgs`?
>
> I don't think we can just remove the `mov` part.
> 
> I don't really understand how to interpret that. We have
> 
>         __ mov(tmp3, dst.base());
>         store_check(masm, tmp3, dst);
> 
> do you suggest we write
> 
>         store_check(masm, tmp3, dst);
> 
> ? But then, `tmp3` is not set to anything meaningful yet. Or 
> 
>         store_check(masm, dst.base(), dst);
> 
> But that is exactly the else-branch. Moreover, I've tried to guard and it makes some test fails. For instance, we can come from
> 
> CardTableBarrierSetAssembler::oop_store_at (cardTableBarrierSetAssembler_x86.cpp:184)
> CardTableBarrierSetAssembler::store_at (cardTableBarrierSetAssembler_x86.cpp:90)
> MacroAssembler::store_heap_oop (macroAssembler_x86.cpp:5515)
> do_oop_store (templateTable_x86.cpp:148)
> (called, for instance, from TemplateTable::putfield_or_static_helper (templateTable_x86.cpp:2964))
> 
> that doesn't need `InlineTypePassFieldsAsArgs` but will give `tmp3` and if we don't use it here (for instance, if I replace the condition by `InlineTypePassFieldsAsArgs && tmp3 != noreg`), we get various test failures.

I don't understand why it's not a problem on mainline, tho. And I agree it's weird. I've looked, but it's hard to investigate why something works rather than not (on a somewhat diverging codebase). I spent some time on that, but eventually decided it's not that critical, as long as we have a way to make it work for Valhalla too.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1824#discussion_r2634193195