> Hi, > > Currently, C2 eagerly expands an oop leaf field of a value class if that leaf > field has its type being a value class, this happens recursively, as long as > there is no cycle. This eagerness may help the compiler have a better chance > optimizing merging value types, as the `InlineTypeNode` is pushed down > through the `Phi`s, allows C2 to fold the loads better. However, this > expansion is unbounded, which means at any parsing or IGVN step, the compiler > may emit an unbounded amount of nodes. > > This PR removes this eagerness, that is, it makes the compiler not try > expanding a value object subfield, unless the field is loaded. This removes > the unbounded nature of value object expansion, it also makes the compiler > emit less node, which alleviate the pressure on other optimizations in > extreme cases. > > In most case, this does not change the behaviour of C2, as value objects are > aggressively flattened inside other value objects, unless their > `LooselyConsistentValue` mismatch, which is not an issue for JEP 401. It also > means that, we have no choice but stopping flattening at this level, because > going even 1 level deeper may lead to issues in extreme cases, as a value > object can have hundreds of leaf fields. This is demonstrated in the added > regression test. `V1` is fully flattened, `V0` flattens only its first field, > so if we also expand the leaf fields of `V0`, we will expand the object > fully, leading to an explosion of node number. > > There are mutliple test failures due to the compiler not able to fold through > a load from a `Phi` of `InlineTypeNode`s. This will need fixing after JEP 401. > > I encounted a tricky issue with load folding from a value object, which is > exposed by this patch. That is, when we push a `CastPP` through an > `InlineTypeNode`, we need to cast all of its oop field that is null-free to > not null. This is because we inject the information that those fields are > null-free into the loads, but the inputs of the `InlineTypeNode` can be > `null`, because the `InlineTypeNode` itself is nullable. As a result, when > the load looks through the `Cast` to the `InlineTypeNode`, it misses the > dependency of the `Cast` that it just looked through, dropping the constraint > that the loaded value must be not null. As a result, it can be scheduled > before the null check, resulting in a crash. The fix is to unconditionally > push a cast to not-null through its `InlineTypeNode` input, to add `CastPP` > of null-free inputs to not-null, and to remove the ability of > `see_through_inlin...
Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision: Add reminder to reenable verifications ------------- Changes: - all: https://git.openjdk.org/valhalla/pull/2474/files - new: https://git.openjdk.org/valhalla/pull/2474/files/a3d2aa7d..e533df3e Webrevs: - full: https://webrevs.openjdk.org/?repo=valhalla&pr=2474&range=02 - incr: https://webrevs.openjdk.org/?repo=valhalla&pr=2474&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/valhalla/pull/2474.diff Fetch: git fetch https://git.openjdk.org/valhalla.git pull/2474/head:pull/2474 PR: https://git.openjdk.org/valhalla/pull/2474
