[lworld] RFR: 8353717: [lworld] Complete merge of 8350756
Christian Hagedorn
chagedorn at openjdk.org
Thu Apr 10 07:27:38 UTC 2025
On Thu, 10 Apr 2025 06:11:25 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:
> This patch fixes some failures after merging JDK-25+14 in:
>
> 1. [JDK-8333393](https://bugs.openjdk.org/browse/JDK-8333393) improved the "split Phi through MergeMem" optimization and was merged in:
> 1. This should fix [JDK-8327465](https://bugs.openjdk.org/browse/JDK-8327465) (it does not reproduce anymore but I think it's still an issue before this patch): When adding `Phi` nodes to the `OuterStripMinedLoop` node, it should match the number of `Phi` nodes at the `CountedLoop` node. However, during IGVN, we could split some phis of the `CountedLoop` node through `MergeMems` which created more phi nodes at the `OuterStripMinedLoop` node. But these phis could not be split further and we hit an assertion failure due to this phi number mismatch. Only with JDK-8333393, we can further split these phis at the `OuterStripMinedLoop` node and we no longer hit the assert. I've therefore closed JDK-8327465 as a dup.
> 2. The merge of JDK-8333393 was not correct:
> 1. We ended up splitting fewer phi nodes through mergemems as before and thus hitting case `1.` even more often.
> 2. The Valhalla specific "Always split `Phi` when only having `MergeMem` inputs" optimization was broken. This leads to some IR rule failures since the splitting optimization is no longer performed which blocked some loop optimizations (e.g. turning a loop into a counted one and then apply more optimizations).
>
> I fixed both issues.
> 2. With JDK-25+14, some tests to check Loop Unswitching started to fail which matched on the number of `CountedLoop` nodes. The reason is that we can now eliminate one of the unswitched loop versions which we previously could not. Therefore, the number of `CountedLoop` nodes is one less. To fix that, I've changed the IR matching to `CCP1` where we still have both unswitched loop versions. This required the following:
> 1. I had to adjust `INLINE_ARRAY_NULL_GUARD` to match on ideal graphs instead of the opto assembly. This can easily be done since all information is there in the ideal graph.
> 2. I removed the `applyIf*` conditions for `UseG1GC` because we are not running any scenario with another GC.
>
> Thanks,
> Christian
Thanks for the review!
-------------
PR Comment: https://git.openjdk.org/valhalla/pull/1426#issuecomment-2791802452
More information about the valhalla-dev
mailing list