On Fri, 19 Dec 2025 08:35:36 GMT, Marc Chevalier <[email protected]> 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

Reply via email to