> 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:

  Rename ConstraintCastNode::ideal_inline_type_node

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

Changes:
  - all: https://git.openjdk.org/valhalla/pull/2474/files
  - new: https://git.openjdk.org/valhalla/pull/2474/files/c3a50bf4..a3d2aa7d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=valhalla&pr=2474&range=01
 - incr: https://webrevs.openjdk.org/?repo=valhalla&pr=2474&range=00-01

  Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 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

Reply via email to