Re: [patch] PR54149: fix data race in LIM pass

[Aldy Hernandez]

ok with

   && gimple_assign_lhs_ptr (loc->stmt) == loc->ref

instead.  Let's hope we conservatively catch all writes to ref this way (which
is what we need, right)?

Yes.

Thanks.  Committing the attached patch.


        PR middle-end/54149
        * tree-ssa-loop-im.c (execute_sm_if_changed_flag_set): Only set
        flag for writes.

diff --git a/gcc/testsuite/gcc.dg/simulate-thread/speculative-store-4.c 
b/gcc/testsuite/gcc.dg/simulate-thread/speculative-store-4.c
new file mode 100644
index 0000000..59f81b7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/simulate-thread/speculative-store-4.c
@@ -0,0 +1,54 @@
+/* { dg-do link } */
+/* { dg-options "--param allow-store-data-races=0" } */
+/* { dg-final { simulate-thread } } */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "simulate-thread.h"
+
+/* PR 54139 */
+/* Test that speculative stores do not happen for --param
+   allow-store-data-races=0.  */
+
+int g_13=1, insns=1;
+
+__attribute__((noinline))
+void simulate_thread_main()
+{
+  int l_245;
+
+  /* Since g_13 is unilaterally set positive above, there should be
+     no store to g_13 below.  */
+  for (l_245 = 0; l_245 <= 1; l_245 += 1)
+    for (; g_13 <= 0; g_13 = 1)
+      ;
+}
+
+int main()
+{
+  simulate_thread_main ();
+  simulate_thread_done ();
+  return 0;
+}
+
+void simulate_thread_other_threads ()
+{
+  ++g_13;
+  ++insns;
+}
+
+int simulate_thread_step_verify ()
+{
+  return 0;
+}
+
+int simulate_thread_final_verify ()
+{
+  if (g_13 != insns)
+    {
+      printf("FAIL: g_13 was incorrectly cached\n");
+      return 1;
+    }
+  return 0;
+}
diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
index 0f61631..67cab3a 100644
--- a/gcc/tree-ssa-loop-im.c
+++ b/gcc/tree-ssa-loop-im.c
@@ -2113,9 +2113,14 @@ execute_sm_if_changed_flag_set (struct loop *loop, 
mem_ref_p ref)
       gimple_stmt_iterator gsi;
       gimple stmt;
 
-      gsi = gsi_for_stmt (loc->stmt);
-      stmt = gimple_build_assign (flag, boolean_true_node);
-      gsi_insert_after (&gsi, stmt, GSI_CONTINUE_LINKING);
+      /* Only set the flag for writes.  */
+      if (is_gimple_assign (loc->stmt)
+         && gimple_assign_lhs_ptr (loc->stmt) == loc->ref)
+       {
+         gsi = gsi_for_stmt (loc->stmt);
+         stmt = gimple_build_assign (flag, boolean_true_node);
+         gsi_insert_after (&gsi, stmt, GSI_CONTINUE_LINKING);
+       }
     }
   VEC_free (mem_ref_loc_p, heap, locs);
   return flag;