[gcc r17-3003] phiopt/cselim: Allow a load between the load and the store before the condition [PR126580]

Andrea Pinski via Gcc-cvs <[email protected]> Thu, 6 Aug 2026 05:03:44 +0000 (GMT)
Newsgroups gmane.comp.gcc.cvs
Message-ID <[email protected]>
https://gcc.gnu.org/g:cd65161b78ececc629183d210b56142c6653c092

commit r17-3003-gcd65161b78ececc629183d210b56142c6653c092
Author: Andrea Pinski <[email protected]>
Date:   Sun Aug 2 14:32:51 2026 -0700

    phiopt/cselim: Allow a load between the load and the store before the condition [PR126580]
    
    In cond_store_replacement_limited, we currently reject any load after the
    store; this was done as a simple way out but we can do better and just not
    remove the store.
    
    That is we have:
    ```
    MEM0 = val;
    _1 = MEM1;
    if (_8)
      MEM0 = val2;
    ```
    cselim (non limited) and ifcvt would turn this info:
    ```
    MEM0 = val;
    _1 = MEM1;
    _2 = _8 ? val : val2;
    MEM0 = _2;
    ```
    So it would be a good idea to do it in limited too and not depend on the
    non-trapping part of cselim. So in the case of the testcases we can remove
    the conditional fully and just have 2 stores.  DSE will remove the
    first store if the load does not alias too.
    
    Bootstrapped and tested on x86_64-linux-gnu.
    
            PR tree-optimization/126580
    
    gcc/ChangeLog:
    
            * tree-ssa-phiopt.cc (cond_store_replacement_limited): Allow
            a load before the store; not removing the store.
    
    gcc/testsuite/ChangeLog:
    
            * gcc.dg/tree-ssa/pr126580-1.c: New test.
            * gcc.dg/tree-ssa/pr126580-2.c: New test.
    
    Signed-off-by: Andrea Pinski <[email protected]>

Diff:
---
 gcc/testsuite/gcc.dg/tree-ssa/pr126580-1.c | 16 ++++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/pr126580-2.c | 15 +++++++++++++++
 gcc/tree-ssa-phiopt.cc                     | 16 +++++++++++-----
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr126580-1.c b/gcc/testsuite/gcc.dg/tree-ssa/pr126580-1.c
new file mode 100644
index 000000000000..98115c0ac756
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr126580-1.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-cselim -fdump-tree-phiopt1-details" } */
+
+int *sink(int*);
+void f(int a, int c, int d, int *e)
+{
+  e = sink(&a);
+  a = d;
+  c = *e;
+  c += a;
+  if (c)
+    a = d|c;
+  sink(&a);
+}
+
+/* { dg-final { scan-tree-dump "Conditional store replacement" "phiopt1" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr126580-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr126580-2.c
new file mode 100644
index 000000000000..5e0cd96b3710
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr126580-2.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-cselim -fdump-tree-phiopt1-details" } */
+
+
+void f1(int *a, int c, int d, int *e)
+{
+  *a = d;
+  c = *e;
+  c += *a;
+  int t = d|c;
+  if (c)
+    *a = t;
+}
+
+/* { dg-final { scan-tree-dump "Conditional store replacement" "phiopt1" } } */
diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 2195be6c503c..129a25abffbb 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -3689,17 +3689,22 @@ cond_store_replacement_limited (basic_block middle_bb, basic_block join_bb,
 	  tree vuse = gimple_vuse (store_middle);
 	  imm_use_iterator iter;
 	  gimple *use_stmt;
-	  /* There can't be any loads between the store and
-	     the previous store as that might depend on the store.
-	     FIXME: use alias oracle to check dependancies.  */
+	  bool has_load = false;
+	  /* If there is a load, then just reuse the value and not
+	     remove the old store as that might be used by the load.  */
 	  FOR_EACH_IMM_USE_STMT (use_stmt, iter, vuse)
 	    {
 	      if (use_stmt != store_middle
 		  && use_stmt != vphi)
-		return false;
+		{
+		  has_load = true;
+		  break;
+		}
 	    }
 	  other_rhs = gimple_assign_rhs1 (vdef_before);
-	  beforestore = vdef_before;
+	  /* If there is no load, then keep the reference to the store stmt.  */
+	  if (!has_load)
+	    beforestore = vdef_before;
 	}
     }
   /*
@@ -3788,6 +3793,7 @@ cond_store_replacement_limited (basic_block middle_bb, basic_block join_bb,
   gsi_remove (&gsi, true);
   release_defs (store_middle);
 
+  /* Remove the store before the conditional if possible.  */
   if (beforestore)
     {
       gsi = gsi_for_stmt (beforestore);