> 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 scala...
Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision: Recycling ------------- Changes: - all: https://git.openjdk.org/valhalla/pull/2547/files - new: https://git.openjdk.org/valhalla/pull/2547/files/0b42d25b..746cd8f3 Webrevs: - full: https://webrevs.openjdk.org/?repo=valhalla&pr=2547&range=01 - incr: https://webrevs.openjdk.org/?repo=valhalla&pr=2547&range=00-01 Stats: 13 lines in 1 file changed: 0 ins; 10 del; 3 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
