[gcc r13-8740] [PR114415][scheduler]: Fixing wrong code generation

[gcc r13-8740] [PR114415][scheduler]: Fixing wrong code generation

[Vladimir Makarov]

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

commit r13-8740-ge30211cb0b3a2b88959e9bc40626a17461de52de
Author: Vladimir N. Makarov <vmakarov@redhat.com>
Date:   Thu Apr 4 16:04:04 2024 -0400

    [PR114415][scheduler]: Fixing wrong code generation
    
      For the test case, the insn scheduler (working for live range
    shrinkage) moves insns modifying stack memory before an insn reserving
    the stack memory. Comments in the patch contains more details about
    the problem and its solution.
    
    gcc/ChangeLog:
    
            PR rtl-optimization/114415
            * sched-deps.cc (add_insn_mem_dependence): Add memory check for mem argument.
            (sched_analyze_1): Treat stack pointer modification as memory read.
            (sched_analyze_2, sched_analyze_insn): Add memory guard for processing pending_read_mems.
            * sched-int.h (deps_desc): Add comment to pending_read_mems.
    
    gcc/testsuite/ChangeLog:
    
            PR rtl-optimization/114415
            * gcc.target/i386/pr114415.c: New test.

Diff:
---
 gcc/sched-deps.cc                        | 49 +++++++++++++++++++++-----------
 gcc/sched-int.h                          |  4 ++-
 gcc/testsuite/gcc.target/i386/pr114415.c | 47 ++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 17 deletions(-)

diff --git a/gcc/sched-deps.cc b/gcc/sched-deps.cc
index 2aa6623ad2ea..2104895f3009 100644
--- a/gcc/sched-deps.cc
+++ b/gcc/sched-deps.cc
@@ -1735,7 +1735,7 @@ add_insn_mem_dependence (class deps_desc *deps, bool read_p,
   insn_node = alloc_INSN_LIST (insn, *insn_list);
   *insn_list = insn_node;
 
-  if (sched_deps_info->use_cselib)
+  if (sched_deps_info->use_cselib && MEM_P (mem))
     {
       mem = shallow_copy_rtx (mem);
       XEXP (mem, 0) = cselib_subst_to_values_from_insn (XEXP (mem, 0),
@@ -2458,6 +2458,25 @@ sched_analyze_1 (class deps_desc *deps, rtx x, rtx_insn *insn)
 			       FIRST_STACK_REG);
 	}
 #endif
+      if (!deps->readonly && regno == STACK_POINTER_REGNUM)
+	{
+	  /* Please see PR114115.  We have insn modifying memory on the stack
+	     and not addressed by stack pointer and we have insn reserving the
+	     stack space.  If we move the insn modifying memory before insn
+	     reserving the stack space, we can change memory out of the red
+	     zone.  Even worse, some optimizations (e.g. peephole) can add
+	     insns using temporary stack slots before insn reserving the stack
+	     space but after the insn modifying memory.  This will corrupt the
+	     modified memory.  Therefore we treat insn changing the stack as
+	     reading unknown memory.  This will create anti-dependence.  We
+	     don't need to treat the insn as writing memory because GCC by
+	     itself does not generate code reading undefined stack memory.  */
+	  if ((deps->pending_read_list_length + deps->pending_write_list_length)
+	      >= param_max_pending_list_length
+	      && !DEBUG_INSN_P (insn))
+	    flush_pending_lists (deps, insn, true, true);
+	  add_insn_mem_dependence (deps, true, insn, dest);
+	}
     }
   else if (MEM_P (dest))
     {
@@ -2498,10 +2517,11 @@ sched_analyze_1 (class deps_desc *deps, rtx x, rtx_insn *insn)
 	  pending_mem = deps->pending_read_mems;
 	  while (pending)
 	    {
-	      if (anti_dependence (pending_mem->element (), t)
-		  && ! sched_insns_conditions_mutex_p (insn, pending->insn ()))
-		note_mem_dep (t, pending_mem->element (), pending->insn (),
-			      DEP_ANTI);
+	      rtx mem = pending_mem->element ();
+	      if (REG_P (mem)
+		  || (anti_dependence (mem, t)
+		      && ! sched_insns_conditions_mutex_p (insn, pending->insn ())))
+		note_mem_dep (t, mem, pending->insn (), DEP_ANTI);
 
 	      pending = pending->next ();
 	      pending_mem = pending_mem->next ();
@@ -2637,12 +2657,10 @@ sched_analyze_2 (class deps_desc *deps, rtx x, rtx_insn *insn)
 	    pending_mem = deps->pending_read_mems;
 	    while (pending)
 	      {
-		if (read_dependence (pending_mem->element (), t)
-		    && ! sched_insns_conditions_mutex_p (insn,
-							 pending->insn ()))
-		  note_mem_dep (t, pending_mem->element (),
-				pending->insn (),
-				DEP_ANTI);
+		rtx mem = pending_mem->element ();
+		if (MEM_P (mem) && read_dependence (mem, t)
+		    && ! sched_insns_conditions_mutex_p (insn, pending->insn ()))
+		  note_mem_dep (t, mem, pending->insn (), DEP_ANTI);
 
 		pending = pending->next ();
 		pending_mem = pending_mem->next ();
@@ -3026,8 +3044,7 @@ sched_analyze_insn (class deps_desc *deps, rtx x, rtx_insn *insn)
 	  while (pending)
 	    {
 	      if (! sched_insns_conditions_mutex_p (insn, pending->insn ()))
-		add_dependence (insn, pending->insn (),
-				REG_DEP_OUTPUT);
+		add_dependence (insn, pending->insn (),	REG_DEP_OUTPUT);
 	      pending = pending->next ();
 	      pending_mem = pending_mem->next ();
 	    }
@@ -3036,10 +3053,10 @@ sched_analyze_insn (class deps_desc *deps, rtx x, rtx_insn *insn)
 	  pending_mem = deps->pending_read_mems;
 	  while (pending)
 	    {
-	      if (MEM_VOLATILE_P (pending_mem->element ())
+	      rtx mem = pending_mem->element ();
+	      if (MEM_P (mem) && MEM_VOLATILE_P (mem)
 		  && ! sched_insns_conditions_mutex_p (insn, pending->insn ()))
-		add_dependence (insn, pending->insn (),
-				REG_DEP_OUTPUT);
+		add_dependence (insn, pending->insn (),	REG_DEP_OUTPUT);
 	      pending = pending->next ();
 	      pending_mem = pending_mem->next ();
 	    }
diff --git a/gcc/sched-int.h b/gcc/sched-int.h
index 97b7d2d319bc..32a9db960c6a 100644
--- a/gcc/sched-int.h
+++ b/gcc/sched-int.h
@@ -471,7 +471,9 @@ public:
   /* An INSN_LIST containing all insns with pending read operations.  */
   rtx_insn_list *pending_read_insns;
 
-  /* An EXPR_LIST containing all MEM rtx's which are pending reads.  */
+  /* An EXPR_LIST containing all MEM rtx's which are pending reads.  The list
+     can contain stack pointer instead of memory.  This is a special case (see
+     sched-deps.cc::sched_analyze_1).  */
   rtx_expr_list *pending_read_mems;
 
   /* An INSN_LIST containing all insns with pending write operations.  */
diff --git a/gcc/testsuite/gcc.target/i386/pr114415.c b/gcc/testsuite/gcc.target/i386/pr114415.c
new file mode 100644
index 000000000000..90c16a5128f0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr114415.c
@@ -0,0 +1,47 @@
+/* { dg-do run  { target int128 } } */
+/* { dg-options "-Oz -fno-dce -fno-forward-propagate -flive-range-shrinkage -fweb -Wno-psabi" } */
+
+typedef unsigned V __attribute__((vector_size (64)));
+typedef unsigned __int128 W __attribute__((vector_size (64)));
+__int128 i;
+V v;
+W w;
+
+W
+bar2 (V, W c)
+{
+  v |= (V){c[3]};
+  i = 0;
+  return c;
+}
+
+W
+bar1 (W, W d)
+{
+  v[0] %= 2;
+  return d;
+}
+
+W
+bar0 (V a, W b)
+{
+  W t = bar2 ((V) { }, w);
+  (void)bar2 ((V){b[0] - a[0]}, (W){});
+  b += bar1 (t, (W){1});
+  a[5] &= __builtin_mul_overflow_p (a[0], i, 0u);
+  return (W) a + b;
+}
+
+int
+main ()
+{
+  W x = bar0 ((V) { }, (W) { });
+  if (x[0] != 1)
+    __builtin_abort();
+  if (x[1])
+    __builtin_abort();
+  if (x[2])
+    __builtin_abort();
+  if (x[3])
+    __builtin_abort();
+}