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

Tobias Hartmann thartmann at openjdk.org
Fri Dec 19 08:59:56 UTC 2025


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

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

I was suggesting that we should do what mainline does if `!InlineTypePassFieldsAsArgs`, would that make sense?

I.e. do `store_check(masm, dst.base(), dst);`

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

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


More information about the valhalla-dev mailing list