[lworld] Integrated: 8293541: [lworld] IR verification fails for TestLWorld::test109_sharp and test110_sharp

[Christian Hagedorn]

On Thu, 7 Mar 2024 07:36:41 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> `test109_sharp()` and `test110_sharp()` originally failed because we have no longer beeen able to apply Loop Predication due to the following merge mistake (left previous, middle merged, right new code from mainline):
> 
> ![image](https://github.com/openjdk/valhalla/assets/17833009/31c5065a-7e8a-4fcd-bc23-241176cfe9a6)
> 
> This changed the application of EA for these tests in such a way that we need to apply EA twice (iteratively) to remove the wrapping box object and the inline type allocation. This had the side effect that we missed to re-add a memory phi to the IGVN worklist to apply an optimization to get rid of a diamond-if control flow which blocks Loop Predication. IR matching failed because we could not find the corresponding predicate trap for an expected Hoisted Check Predicate.
> 
> This was also a problem in mainline but we had no IR test to catch this. [JDK-8314997](https://bugs.openjdk.org/browse/JDK-8314997) fixed this for mainline and once it was merged with Valhalla, `test109_sharp()` and `test110_sharp()` would now work again.
> 
> But I think we should still fix the merge mistake to eagerly try to remove inline type allocations, regardless of their escape/replaceable status as it was originally intended before the merge. This allows us to get rid of some extra EA iterations.
> 
> However, fixing this merge mistake caused an assertion failure later in EA which checked that safepoints do not have inline type allocation uses (they should be replaced by `SafePointScalarObjectNodes`). To fix that, we need to make sure that any inline type pointer phi node created for an inline type field here (`create_scalarized_object_description()` -> `value_from_mem()` -> `value_from_mem_phi()`):
> 
> https://github.com/openjdk/valhalla/blob/814a5e93cb392a77a43550bf8dab5ab9d52bdcfd/src/hotspot/share/opto/macro.cpp#L378-L379
> 
> when trying to create a `SafePointScalarObjectNode` for a safepoint use is eagerly replaced by an `InlineTypeNode`. To achieve that, I've added a call to `push_inline_types_through()` (renamed to `push_inline_types_down()`) which was only used in `PhiNode::Ideal()` before. I've applied some refactoring and renaming to share that code with EA
> 
> I've tried out some additional assertions in EA that an inline type pointer phi should already be replaced by an `InlineTypeNode` if possible. But that failed. I think we are missing some optimization opportunities here. As this might be out of scope of this bug, I suggest to file a separate RFE to investigate further....

This pull request has now been integrated.

Changeset: 7749f995
Author:    Christian Hagedorn <chagedorn at openjdk.org>
Committer: Tobias Hartmann <thartmann at openjdk.org>
URL:       https://git.openjdk.org/valhalla/commit/7749f995f9377f358200e9e49bf139b6b99286eb
Stats:     190 lines in 5 files changed: 107 ins; 64 del; 19 mod

8293541: [lworld] IR verification fails for TestLWorld::test109_sharp and test110_sharp

Reviewed-by: thartmann

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

PR: https://git.openjdk.org/valhalla/pull/1038