[Bug tree-optimization/112402] New: [11/12/13/14 Regression] Path splitting causes if-conversion miss

[Bug tree-optimization/112402] New: [11/12/13/14 Regression] Path splitting causes if-conversion miss

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

            Bug ID: 112402
           Summary: [11/12/13/14 Regression] Path splitting causes
                    if-conversion miss
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pinskia at gcc dot gnu.org
            Blocks: 94274
  Target Milestone: ---

Similar to PR 68541 except instead of having a diamond, we just have a
triangle.

Testcase:
```
int
foo(signed char *p, int n)
{
  int s = 0;
  int i;

  for (i = 0; i < n; i++) {
    int t;
    if (p[i] >= 0)
      t=1;
    else
      t=-1;
    s+=t;
  }

  return s;
}
```

I noticed this while working on fixing PR 94274.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94274
[Bug 94274] fold phi whose incoming args are defined from binary operations

[Bug tree-optimization/112402] [11/12/13/14 Regression] Path splitting causes if-conversion miss

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.5

[Bug tree-optimization/112402] [11/12/13/14 Regression] Path splitting causes if-conversion miss

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
So the question is whether we should do more if-conversion on GIMPLE then (or
axe path splitting or move it to RTL)

[Bug tree-optimization/112402] [11/12/13/14 Regression] Path splitting causes if-conversion miss

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The orginal threads about patch splitting:
https://gcc.gnu.org/legacy-ml/gcc/2015-03/msg00057.html
https://gcc.gnu.org/pipermail/gcc/2015-May/217495.html

> or axe path splitting or move it to RTL
Jeff had a similar idea back in 2016 (I would assume while he was looking into
PR 68541):
https://gcc.gnu.org/pipermail/gcc-patches/2016-February/441651.html :
```
And if that's the case, then we may really be looking at something that 
belongs at the RTL level rather than at the tree/gimple level.  Sadly, 
it's harder to do things like duplicate blocks at the RTL level.
```
and also mentioned in (which was the heustrics added to fix PR 68541):
https://gcc.gnu.org/pipermail/gcc-patches/2016-February/441841.html

Maybe this is something for GCC 15 ... to move to this to RTL right during the
loop optimization phase after pass_rtl_doloop. There is still a CSE and DSE/DCE
passes afterwards too.

[Bug tree-optimization/112402] [11/12/13/14 Regression] Path splitting causes if-conversion miss

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org
   Last reconfirmed|                            |2023-11-09
     Ever confirmed|0                           |1

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have a patch which moves split paths to RTL like it should be.  Waiting on
legal though.

[Bug tree-optimization/112402] [11/12/13/14 Regression] Path splitting causes if-conversion miss

[law at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
                 CC|                            |law at gcc dot gnu.org

[Bug tree-optimization/112402] [12/13/14/15 Regression] Path splitting causes if-conversion miss

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.5                        |12.5

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 11 branch is being closed.

[Bug tree-optimization/112402] [12/13/14/15 Regression] Path splitting causes if-conversion miss

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> I have a patch which moves split paths to RTL like it should be.  Waiting on
> legal though.

I am just going to do the simple patch here for ifcvt here.

[Bug tree-optimization/112402] [12/13/14/15 Regression] Path splitting causes if-conversion miss

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have a patch which improves/rewrites (and much needed simplification) the
heuristic in path splitting.

[Bug tree-optimization/112402] [12/13/14/15 Regression] Path splitting causes if-conversion miss

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |https://gcc.gnu.org/piperma
                   |                            |il/gcc-patches/2024-Septemb
                   |                            |er/662150.html

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Submitted:
https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662150.html

[Bug tree-optimization/112402] [12/13/14/15 Regression] Path splitting causes if-conversion miss

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:b2b20b277988ab9ddb6ea82141075147b7b98f74

commit r15-3436-gb2b20b277988ab9ddb6ea82141075147b7b98f74
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Mon Sep 2 21:34:53 2024 -0700

    split-path: Improve ifcvt heurstic for split path [PR112402]

    This simplifies the heurstic for split path to see if the join
    bb is a ifcvt candidate.
    For the predecessors bbs need either to be empty or only have one
    statement in them which could be a decent ifcvt candidate.
    The previous heurstics would miss that:
    ```
    if (a) goto B else goto C;
    B:  goto C;
    C:
    c = PHI<d,e>
    ```

    Would be a decent ifcvt candidate. And would also miss:
    ```
    if (a) goto B else goto C;
    B: d = f + 1;  goto C;
    C:
    c = PHI<d,e>
    ```

    Also since currently the max number of cmovs being able to produced is 3,
we
    should only assume `<= 3` phis can be ifcvt candidates.

    The testcase changes for split-path-6.c is that lookharder function
    is a true ifcvt case where we would get cmov as expected; it looks like it
    was not a candidate when the heurstic was added but became one later on.
    pr88797.C is now rejected via it being an ifcvt candidate rather than being
about
    DCE/const prop.

    The rest of the testsuite changes are just slight change in the dump,
    removing the "*diamnond" part as it was removed from the print.

    Bootstrapped and tested on x86_64.

            PR tree-optimization/112402

    gcc/ChangeLog:

            * gimple-ssa-split-paths.cc (poor_ifcvt_pred): New function.
            (is_feasible_trace): Remove old heurstics for ifcvt cases.
            For num_stmts <=1 for both pred check poor_ifcvt_pred on both
            pred.

    gcc/testsuite/ChangeLog:

            * gcc.dg/tree-ssa/split-path-11.c: Update scan.
            * gcc.dg/tree-ssa/split-path-2.c: Update scan.
            * gcc.dg/tree-ssa/split-path-5.c: Update scan.
            * gcc.dg/tree-ssa/split-path-6.c: Update scan.
            * g++.dg/tree-ssa/pr88797.C: Update scan.
            * gcc.dg/tree-ssa/split-path-13.c: New test.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>

[Bug tree-optimization/112402] [12/13/14/15 Regression] Path splitting causes if-conversion miss

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.5                        |15.0
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.

[Bug tree-optimization/112402] [12/13/14/15 Regression] Path splitting causes if-conversion miss

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112402

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #9)
> Fixed.

I should mention this is not worth fixing on the release branches. Nobody else
noticed this until I found it.