On Tue, 26 May 2026 08:38:00 GMT, Quan Anh Mai <[email protected]> wrote:
>> 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... > > Quan Anh Mai has updated the pull request incrementally with one additional > commit since the last revision: > > Rename ConstraintCastNode::ideal_inline_type_node > Ah, I missed that. Maybe add a TODO comment so we don't miss to re-enable > these or add it to the This should be Integer comment. Done. ------------- PR Comment: https://git.openjdk.org/valhalla/pull/2474#issuecomment-4551555415
