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 inside an `InlineType` (that will be scalarized in safepoint later,
thanks to `InlineTypeNode` logic). The logic is quite simple: we iterate over
the Java arguments, and when it is scalarized, we gather the pieces using the
type to know how many fields to get. Otherwise, the next input is the argument
by itself. Doing so, we can see that the opto assembly looks more reasonable:
19b call,static wrapper for: new_array_blob (C2 runtime)
# ...::test @ bci:54 (line 37) L[0]=RBP L[1]=rsp + #4 L[2]=#ScObj0
L[3]=#ScObj1 L[4]=#ScObj2 L[5]=#ScObj3 STK[0]=#ScObj0 STK[1]=#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}
The bytecode is given a stack containing the buffered `MyValue` and the
`IntVector`: all is fine!
Thanks,
Marc
---------
- [x] I confirm that I make this contribution in accordance with the [OpenJDK
Interim AI Policy](https://openjdk.org/legal/ai).
-------------
Commit messages:
- Add simple run
- A new hope
Changes: https://git.openjdk.org/valhalla/pull/2547/files
Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=2547&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8385886
Stats: 105 lines in 2 files changed: 103 ins; 0 del; 2 mod
Patch: https://git.openjdk.org/valhalla/pull/2547.diff
Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2547/head:pull/2547
PR: https://git.openjdk.org/valhalla/pull/2547
