[cxx-mem-model] disallow load data races (1 of some)

[Aldy Hernandez <aldyh@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]

This is my first stab at disallowing load data races that happen when we 
cache the value of a global.  I would appreciate input before 
committing, especially on the RTL bits, cause it's been quite a while 
since I typed d-e-b-u-g-_-r-t-x. :-)

In the example below we usually hoist "global" into a register or 
temporary to avoid reading from it at each step.  This would cause a 
race if another thread had modified "global" in between iterations.

	for (x=0; x< 5; x++)
		sum[x] =  global;

As expected, multiple passes can cause the same end result, so plugging 
the problem involves multiple places.  First, loop invariant movement 
moves the invariant.  Barring that, PRE moves the load, and even if we 
get past the SSA passes, rtl-CSE pulls the rug from under us.  If that 
weren't enough, the post-reload pass performs CSE over the hard 
registers, and we end up eliminating subsequent loads of "global".  I am 
sure there are many other places, but I'm starting with whatever it 
takes to fix gcc.dg/memmodel/global-hoist.c.

The patch below fixes the test in question with "--param 
allow-load-data-races=0".  What do y'all think?

Thanks.

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 5294 bytes --]

	* tree.h (DECL_THREAD_VISIBLE_P): New.
	* cselib.c (cselib_lookup_mem): Return 0 for globals for
	restrictive memory model.
	* cse.c (hash_rtx_cb): Same.
	* gimple.h (gimple_has_thread_visible_ops): Protoize.
	* gimple.c (gimple_has_thread_visible_ops): New.
	* tree-ssa-loop-im.c (movement_possibility): Disallow when
	avoiding load data races.
	* tree-ssa-pre.c (compute_avail): Same, for globals.

Index: tree-ssa-loop-im.c
===================================================================
--- tree-ssa-loop-im.c	(revision 170745)
+++ tree-ssa-loop-im.c	(working copy)
@@ -377,6 +377,11 @@ movement_possibility (gimple stmt)
       || stmt_could_throw_p (stmt))
     return MOVE_IMPOSSIBLE;
 
+  /* Do not move loads of globals if under a restrictive memory model.  */
+  if (PARAM_VALUE (PARAM_ALLOW_LOAD_DATA_RACES) == 0
+      && gimple_has_thread_visible_ops (stmt))
+    return MOVE_IMPOSSIBLE;
+
   if (is_gimple_call (stmt))
     {
       /* While pure or const call is guaranteed to have no side effects, we
Index: tree.h
===================================================================
--- tree.h	(revision 170745)
+++ tree.h	(working copy)
@@ -3102,6 +3102,10 @@ struct GTY(()) tree_parm_decl {
 #define DECL_THREAD_LOCAL_P(NODE) \
   (VAR_DECL_CHECK (NODE)->decl_with_vis.tls_model >= TLS_MODEL_REAL)
 
+/* Return true if a VAR_DECL is visible from another thread.  */
+#define DECL_THREAD_VISIBLE_P(NODE) \
+  (TREE_STATIC (NODE) && !DECL_THREAD_LOCAL_P (NODE))
+
 /* In a non-local VAR_DECL with static storage duration, true if the
    variable has an initialization priority.  If false, the variable
    will be initialized at the DEFAULT_INIT_PRIORITY.  */
Index: cse.c
===================================================================
--- cse.c	(revision 170745)
+++ cse.c	(working copy)
@@ -2402,6 +2402,19 @@ hash_rtx_cb (const_rtx x, enum machine_m
       }
 
     case MEM:
+      /* Avoid hoisting loads of globals if under a restrictive memory
+	 model.  */
+      if (PARAM_VALUE (PARAM_ALLOW_LOAD_DATA_RACES) == 0)
+	{
+	  tree decl = MEM_EXPR (x);
+	  if (do_not_record_p
+	      && decl && TREE_CODE (decl) == VAR_DECL
+	      && DECL_THREAD_VISIBLE_P (decl))
+	    {
+	      *do_not_record_p = 1;
+	      return 0;
+	    }
+	}
       /* We don't record if marked volatile or if BLKmode since we don't
 	 know the size of the move.  */
       if (do_not_record_p && (MEM_VOLATILE_P (x) || GET_MODE (x) == BLKmode))
Index: cselib.c
===================================================================
--- cselib.c	(revision 170745)
+++ cselib.c	(working copy)
@@ -1190,6 +1190,16 @@ cselib_lookup_mem (rtx x, int create)
   cselib_val *mem_elt;
   struct elt_list *l;
 
+  /* Forget any reads from globals if under a restrictive memory
+     model.  */
+  if (PARAM_VALUE (PARAM_ALLOW_LOAD_DATA_RACES) == 0)
+    {
+      tree decl = MEM_EXPR (x);
+      if (decl && TREE_CODE (decl) == VAR_DECL
+	  && DECL_THREAD_VISIBLE_P (decl))
+	return 0;
+    }
+
   if (MEM_VOLATILE_P (x) || mode == BLKmode
       || !cselib_record_memory
       || (FLOAT_MODE_P (mode) && flag_float_store))
Index: tree-ssa-pre.c
===================================================================
--- tree-ssa-pre.c	(revision 170745)
+++ tree-ssa-pre.c	(working copy)
@@ -4000,6 +4000,12 @@ compute_avail (void)
 	      || stmt_could_throw_p (stmt))
 	    continue;
 
+	  /* Do not move loads of globals if under a restrictive
+	     memory model.  */
+	  if (PARAM_VALUE (PARAM_ALLOW_LOAD_DATA_RACES) == 0
+	      && gimple_has_thread_visible_ops (stmt))
+	    continue;
+
 	  switch (gimple_code (stmt))
 	    {
 	    case GIMPLE_RETURN:
Index: gimple.c
===================================================================
--- gimple.c	(revision 170745)
+++ gimple.c	(working copy)
@@ -2388,6 +2388,36 @@ gimple_rhs_has_side_effects (const_gimpl
   return false;
 }
 
+/* Return true if any of the operands in S are visible from another
+   thread (globals that are not TLS.  */
+/* ?? If this becomes a performance penalty, we should cache this
+   value as we do with volatiles.  */
+
+bool
+gimple_has_thread_visible_ops (const_gimple s)
+{
+  unsigned int i;
+
+  if (is_gimple_debug (s))
+    return false;
+
+  if (is_gimple_assign (s))
+    {
+      /* Skip the LHS.  */
+      i = 1;
+    }
+  else
+    i = 0;
+  for (; i < gimple_num_ops (s); i++)
+    {
+      tree op = gimple_op (s, i);
+      if (op && TREE_CODE (op) == VAR_DECL
+	  && DECL_THREAD_VISIBLE_P (op))
+	return true;
+    }
+  return false;
+}
+
 /* Helper for gimple_could_trap_p and gimple_assign_rhs_could_trap_p.
    Return true if S can trap.  When INCLUDE_MEM is true, check whether
    the memory operations could trap.  When INCLUDE_STORES is true and
Index: gimple.h
===================================================================
--- gimple.h	(revision 170745)
+++ gimple.h	(working copy)
@@ -882,6 +882,7 @@ gimple gimple_build_cond_from_tree (tree
 void gimple_cond_set_condition_from_tree (gimple, tree);
 bool gimple_has_side_effects (const_gimple);
 bool gimple_rhs_has_side_effects (const_gimple);
+bool gimple_has_thread_visible_ops (const_gimple);
 bool gimple_could_trap_p (gimple);
 bool gimple_could_trap_p_1 (gimple, bool, bool);
 bool gimple_assign_rhs_could_trap_p (gimple);