On Mon, 15 Jun 2026 15:12:10 GMT, Marc Chevalier <[email protected]> wrote:

>> The whole problem is there:
>> 
>> https://github.com/openjdk/valhalla/blob/1ca1c8d9301215d43eb0d012cdc42ddbe7549d14/src/hotspot/share/opto/vector.cpp#L204-L210
>> 
>> We are rebuilding the top of the bytecode stack before the call by pushing 
>> the arguments. This will end up as the debug inputs for the allocation in 
>> `kit.box_vector` (throught the chain `GraphKit::box_vector` -> 
>> `GraphKit::set_edges_for_java_call` -> `GraphKit::add_safepoint_edges`). 
>> Having the wrong inputs on the allocation means that if it deopts, the 
>> bytecode state will be restored wrong.
>> 
>> In a case such as in the test, one of the local is scalarized and takes only 
>> one slot on the expression stack (in the bytecode). Meanwhile, for C2, the 
>> value is passed scalarized to the call, so we have inputs for the buffer, 
>> the null marker, and each field. The current implementation has two issues, 
>> the first one is that it iterates with the size of `domain_sig()`, that is 
>> the signature as it appears in the Java code. In the example, that will be 
>> `MyValue` and `IntVector`, thus a length of 2. But the inputs of `call` will 
>> reflect the signature `domain_cc()`, that is with the scalarized calling 
>> convention, which will be in the example: oop buffer for `MyValue`, null 
>> marker, the integer field, and `IntVector` (length = 4). So, the current 
>> code fails to gather all locals entirely. But replacing `domain_sig` with 
>> `domain_cc` isn't enough either. If we do that, we do collect everything, 
>> but they end up as independent values on the bytecode expression stack. This 
>> is visible in the
 opto assembly:
>> 
>> 19b     call,static  wrapper for: new_array_blob (C2 runtime)
>>         # ...::test @ bci:54 (line 45) L[0]=RBP L[1]=rsp + #4 L[2]=#ScObj0 
>> L[3]=#ScObj1 L[4]=#ScObj2 L[5]=#ScObj3 STK[0]=#null STK[1]=#1 STK[2]=rsp + 
>> #4 STK[3]=#ScObj3
>>         # ScObj0 ...$MyValue={ [null marker :-1]=#1 [a :0]=rsp + #4 }
>>         # ScObj1 jdk/incubator/vector/IntVector128={ [payload :0]=rsp + #32 }
>>         # ScObj2 jdk/incubator/vector/IntVector128={ [payload :0]=rsp + #16 }
>>         # ScObj3 jdk/incubator/vector/IntVector128={ [payload :0]=rsp + #48 }
>>         # OopMap {off=416/0x1a0}
>> 
>> Here, it looks like the stack should have 4 slots (instead of 2), and we can 
>> recognize the slots 0, 1 and 2 that forms actually the value `myVal`. But as 
>> a result, after deoptimization, the interpreter is given a state that is not 
>> correct.
>> 
>> So, on top of actually iterating over all the inputs of `call` we also need 
>> to group them insi...
>
> Marc Chevalier has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Recycling

Thanks @merykitty and @dean-long!

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

PR Comment: https://git.openjdk.org/valhalla/pull/2547#issuecomment-4716692594

Reply via email to