This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Volatile MEMs in statement expressions and functions inlined as trees


This patch fixes a few problems we had in volatile mems appearing as
results of statement expressions or of functions inlined as trees.
We'd often read the volatile mems when we shouldn't, or read them too
many times.  The testcase below exposes some of the situations in
which we used to get it wrong, that we get right with this patch.
Unfortunately, I have no idea of how to get this kind of test into the
testsuite.  Verification of correct behavior would require scanning
the assembly output to verify we don't have too many loads.  Even
then, it's tough, because the pointers will be loaded into arbitrary
registers.

inline void f(volatile unsigned char *p)
{
  *p = 0x84; // don't load *p back
}

void g(volatile unsigned char *p)
{
  f(p); // don't load *p back
        // (we used to do it when f was inlined as a tree)
}

volatile int *p, *q, r;

void foo() {
  *p = *q = r; // don't load *p back
}

void bar() {
  *p; // load *p
}

void baz() {
  ({ *p; }); // load *p only once (we used to load it twice)
}

void blaz() {
  ({ *p = *q; }) ; // don't load *p back (we used to load after store)
}

void bazz() {
  char c = ({ *p; }); // load *p once (we used to load it twice)
}

void blazz() {
  char c = ({ *p = *q; }) ; // load *p once after store
}

Here's the patch, bootstrapped and tested on athlon-pc-linux-gnu.  Ok
to install?

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* stmt.c (expand_expr_stmt): Keep last_expr_value non-NULL iff
	we're interested in the result.  Use it to tell whether to
	ignore results of enclosed expressions.
	(expand_start_stmt_expr): Added new argument, and initialize
	last_expr_value accordingly.
	* tree.h (expand_start_stmt_expr): Adjusted declaration.
	* c-common.c (c_expand_expr): Adjust call.
	* expr.c (expand_expr) [EXPR_WFL]: Pass const0_rtx down if
	ignoring the result.

Index: gcc/stmt.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/stmt.c,v
retrieving revision 1.232
diff -u -p -r1.232 stmt.c
--- gcc/stmt.c 2001/12/01 18:42:38 1.232
+++ gcc/stmt.c 2001/12/02 14:25:20
@@ -2149,6 +2149,8 @@ void
 expand_expr_stmt (exp)
      tree exp;
 {
+  bool want_value = last_expr_value != NULL_RTX;
+
   /* If -W, warn about statements with no side effects,
      except for an explicit cast to void (e.g. for assert()), and
      except inside a ({...}) where they may be useful.  */
@@ -2175,7 +2177,7 @@ expand_expr_stmt (exp)
      last_expr_value to get reset.  Therefore, we set last_expr_value
      and last_expr_type *after* calling expand_expr.  */
   last_expr_value = expand_expr (exp,
-				 (expr_stmts_for_value
+				 (want_value && expr_stmts_for_value
 				  ? NULL_RTX : const0_rtx),
 				 VOIDmode, 0);
   last_expr_type = TREE_TYPE (exp);
@@ -2188,7 +2190,7 @@ expand_expr_stmt (exp)
       if (TYPE_MODE (TREE_TYPE (exp)) == VOIDmode)
 	;
       else if (TYPE_MODE (TREE_TYPE (exp)) != BLKmode)
-	copy_to_reg (last_expr_value);
+	last_expr_value = copy_to_reg (last_expr_value);
       else
 	{
 	  rtx lab = gen_label_rtx ();
@@ -2211,6 +2213,14 @@ expand_expr_stmt (exp)
      above.  */
   free_temp_slots ();
 
+  if (! want_value && last_expr_value)
+    {
+      protect_from_queue (last_expr_value, 0);
+      last_expr_value = NULL_RTX;
+    }
+  else if (want_value && ! last_expr_value)
+    last_expr_value = const0_rtx;
+
   emit_queue ();
 }
 
@@ -2336,7 +2346,8 @@ clear_last_expr ()
    The caller must save that value and pass it to expand_end_stmt_expr.  */
 
 tree
-expand_start_stmt_expr ()
+expand_start_stmt_expr (want_value)
+     int want_value;
 {
   tree t;
 
@@ -2347,6 +2358,7 @@ expand_start_stmt_expr ()
   start_sequence_for_rtl_expr (t);
   NO_DEFER_POP;
   expr_stmts_for_value++;
+  last_expr_value = want_value ? const0_rtx : NULL_RTX;
   return t;
 }
 
Index: gcc/tree.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/tree.h,v
retrieving revision 1.290
diff -u -p -r1.290 tree.h
--- gcc/tree.h 2001/11/28 16:55:59 1.290
+++ gcc/tree.h 2001/12/02 14:25:20
@@ -2706,7 +2706,7 @@ extern int type_num_arguments           
 
 extern int in_control_zone_p			PARAMS ((void));
 extern void expand_fixups			PARAMS ((rtx));
-extern tree expand_start_stmt_expr		PARAMS ((void));
+extern tree expand_start_stmt_expr		PARAMS ((int));
 extern tree expand_end_stmt_expr		PARAMS ((tree));
 extern void expand_expr_stmt			PARAMS ((tree));
 extern int warn_if_unused_value			PARAMS ((tree));
Index: gcc/c-common.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/c-common.c,v
retrieving revision 1.275
diff -u -p -r1.275 c-common.c
--- gcc/c-common.c 2001/11/26 23:44:47 1.275
+++ gcc/c-common.c 2001/12/02 14:25:20
@@ -3409,7 +3409,7 @@ c_expand_expr (exp, target, tmode, modif
 	   out-of-scope after the first EXPR_STMT from within the
 	   STMT_EXPR.  */
 	push_temp_slots ();
-	rtl_expr = expand_start_stmt_expr ();
+	rtl_expr = expand_start_stmt_expr (target != const0_rtx);
 	expand_stmt (STMT_EXPR_STMT (exp));
 	expand_end_stmt_expr (rtl_expr);
 	result = expand_expr (rtl_expr, target, tmode, modifier);
Index: gcc/expr.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/expr.c,v
retrieving revision 1.387
diff -u -p -r1.387 expr.c
--- gcc/expr.c 2001/11/28 22:34:07 1.387
+++ gcc/expr.c 2001/12/02 14:25:22
@@ -6509,7 +6509,9 @@ expand_expr (exp, target, tmode, modifie
 	if (EXPR_WFL_EMIT_LINE_NOTE (exp))
 	  emit_line_note (input_filename, lineno);
 	/* Possibly avoid switching back and forth here.  */
-	to_return = expand_expr (EXPR_WFL_NODE (exp), target, tmode, modifier);
+	to_return = expand_expr (EXPR_WFL_NODE (exp),
+				 target || ! ignore ? target : const0_rtx,
+				 tmode, modifier);
 	input_filename = saved_input_filename;
 	lineno = saved_lineno;
 	return to_return;
Index: gcc/cp/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* semantics.c (begin_global_stmt_expr): Adjust
	expand_start_stmt_expr invocation.

Index: gcc/cp/semantics.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/cp/semantics.c,v
retrieving revision 1.230
diff -u -p -r1.230 semantics.c
--- gcc/cp/semantics.c 2001/11/23 19:00:14 1.230
+++ gcc/cp/semantics.c 2001/12/02 15:40:53
@@ -1292,7 +1292,7 @@ begin_global_stmt_expr ()
 
   keep_next_level (1);
   
-  return (last_tree != NULL_TREE) ? last_tree : expand_start_stmt_expr(); 
+  return (last_tree != NULL_TREE) ? last_tree : expand_start_stmt_expr(1); 
 }
 
 /* Finish the STMT_EXPR last begun with begin_global_stmt_expr.  */
Index: gcc/f/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* com.c (ffecom_expr_power_integer_): Adjust
	expand_start_stmt_expr() invocation.

Index: gcc/f/com.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/f/com.c,v
retrieving revision 1.141
diff -u -p -r1.141 com.c
--- gcc/f/com.c 2001/11/18 11:04:48 1.141
+++ gcc/f/com.c 2001/12/02 15:40:54
@@ -5584,7 +5584,7 @@ ffecom_expr_power_integer_ (ffebld expr)
     basetypeof_l_is_int
       = build_int_2 ((TREE_CODE (ltype) == INTEGER_TYPE), 0);
 
-    se = expand_start_stmt_expr ();
+    se = expand_start_stmt_expr (1);
 
     ffecom_start_compstmt ();
 

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                  aoliva@{cygnus.com, redhat.com}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist    *Please* write to mailing lists, not to me

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]