RFR: 8304049: C2 can not merge trivial Ifs due to CastII
Roland Westrelin
roland at openjdk.org
Wed Mar 15 12:16:20 UTC 2023
On Wed, 15 Mar 2023 10:37:03 GMT, Yi Yang <yyang at openjdk.org> wrote:
> Hi can I have a review for this patch? C2 can not apply Split If for the attached trivial case. PhiNode::Ideal removes itself by unique_input but introduces a new CastII
>
> https://github.com/openjdk/jdk/blob/e3777b0c49abb9cc1925f4044392afadf3adef61/src/hotspot/share/opto/cfgnode.cpp#L1470-L1474
>
> https://github.com/openjdk/jdk/blob/e3777b0c49abb9cc1925f4044392afadf3adef61/src/hotspot/share/opto/cfgnode.cpp#L2078-L2079
>
> Therefore we have two Cmp, which is not identical for split_if.
>
> ![image](https://user-images.githubusercontent.com/5010047/225285449-b41dc939-1d3f-45f3-b6d6-a9b9445c2f6a.png)
> (Fig1. Phi#41 is removed during ideal, create CastII#58 then)
>
> ![image](https://user-images.githubusercontent.com/5010047/225285493-30471f1c-97b0-452b-9218-3b5f09f09859.png)
> (Fig2. CmpI#42 and CmpI#23 are different comparisons, they are not identical_backtoback_ifs )
>
> This patch adds Cmp identity to find existing Cmp node, i.e. Cmp#42 is identity to Cmp#23
>
>
> public static void test5(int a, int b){
>
> if( b!=0) {
> int_field = 35;
> } else {
> int_field =222;
> }
>
> if( b!=0) {
> int_field = 35;
> } else {
> int_field =222;
> }
> }
>
>
>
> Test: tier1, application/ctw/modules
Why not do this (and the other similar #12978) by extending the logic where the merge happens in `PhaseIdealLoop::identical_backtoback_ifs()`?
I see a few reasons why that would be preferable:
- if the motivation is only to merge ifs, is it a good thing to transform the graph blindly during GVN when we don't know if the ifs are even candidates for merging?
- having that code far away from where the merge transformation happens makes understanding what's going on harder
- dropping a Cast node is risky, in general, because they sometimes carry a dependence that's required for correctness. So it should be done with care.
-------------
PR: https://git.openjdk.org/jdk/pull/13039
More information about the hotspot-compiler-dev
mailing list