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]

Re: Volatile MEMs in statement expressions and functions inlined as trees


On Dec  5, 2001, Alexandre Oliva <aoliva@redhat.com> wrote:

> FWIW, I've written a new patch that marks the last EXPR_STMT of a
> STMT_EXPR whose value is wanted with TREE_ADDRESSABLE, and then
> arranged for the expr of the EXPR_STMT to be evaluated such that its
> value is wanted.  The expr containing the return value of an inlined
> function is marked similarly.

> I'm testing my patch now (but I'm having some trouble with Ada, whose
> bootstrap compiler I've installed yesterday; I don't know whether it's
> the bootstrap compiler or my patch that are faulty :-(

Here's the patch, bootstrapped on athlon-pc-linux-gnu, and tested
without regressions.  I couldn't test Ada, though, but this should
preserve exactly the original behavior of other front-ends (the reason
why I have introduced deprecated features and didn't touch other
front-ends).  It seems that the compiler I had installed isn't up to
the task of building the version in GCC CVS :-(

The testcase is still under development, but I won't have it finished
today (I have to leave in a few minutes).

Some differences introduced by this patch: we used to expand *all*
expressions with want_value whenever they appeared inside EXPR_STMTs,
which was true for inlined functions and for all expressions inside
({...}).  Now, we only request and save the value of EXPR_STMTs such
as the USE of the return value of the inlined-function STMT_EXPR,
regardless of whether it's actually used, and the last EXPR_STMT of
the COMPOUND_STMT in the STMT_EXPR, if the enclosing STMT_EXPR was
expanded with want_value.  It is assumed that an STMT_EXPR must have a
COMPOUND_STMT as argument, and that this COMPOUND_STMT must start with
a SCOPE_STMT, chained to a sequence of statements that end with
another SCOPE_STMT.  This is true given my analysis of STMT_EXPRs in
the parser, but I have be missing something.  One possible change is
that, if the element before the SCOPE_STMT is not an EXPR_STMT, but
some other kind of statement (say a block or a loop or an if/else), we
won't save its value.  I don't think we did before, either, at least
not consistently, so I don't think it is an important change, if it's
a change at all.  The fact that it has survived an optimized bootstrap
on a machine that uses glibc, with all the inline functions and macros
that use a lot of expression statements seems to be a good indication
that no significant change was introduced in (except perhaps for
volatile memory accesses, which is what the patch is supposed to fix,
after all :-)

Ok to install?

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

	* c-common.h (genrtl_expr_stmt_value): Declare.
	* c-semantics.c (genrtl_goto_stmt): Redirect to...
	(genrtl_goto_stmt_value): ... this new function.  Pass new
	argument down to expand_expr_stmt_value, taking
	TREE_ADDRESSABLE into account.
	* c-common.c (c_expand_expr): Mark the last EXPR_STMT of a
	STMT_EXPR as addressable, i.e., one whose result we want.
	* expr.c (expand_expr): Don't save expression statement value
	of labeled_blocks or loop_exprs.
	* stmt.c (expand_expr_stmt): Redirect to...
	(expand_expr_stmt_value): ... this new function.  Use new
	argument to tell whether to save expression value.
	(expand_end_stmt_expr): Reset last_expr_type and
	last_expr_value if we don't have either.
	* tree-inline.c (declare_return_variable): Mark its use
	statement as addressable.
	* tree.h: Document new use of TREE_ADDRESSABLE.
	(expand_expr_stmt_value): Declare.

Index: gcc/c-common.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/c-common.c,v
retrieving revision 1.278
diff -u -p -r1.278 c-common.c
--- gcc/c-common.c 2001/12/04 22:55:36 1.278
+++ gcc/c-common.c 2001/12/05 17:00:18
@@ -3410,6 +3410,27 @@ c_expand_expr (exp, target, tmode, modif
 	   STMT_EXPR.  */
 	push_temp_slots ();
 	rtl_expr = expand_start_stmt_expr ();
+
+	/* If we want the result of this expression, find the last
+           EXPR_STMT in the COMPOUND_STMT and mark it as addressable.  */
+	if (target != const0_rtx
+	    && TREE_CODE (STMT_EXPR_STMT (exp)) == COMPOUND_STMT
+	    && TREE_CODE (COMPOUND_BODY (STMT_EXPR_STMT (exp))) == SCOPE_STMT)
+	  {
+	    tree expr = COMPOUND_BODY (STMT_EXPR_STMT (exp));
+	    tree last = TREE_CHAIN (expr);
+
+	    while (TREE_CHAIN (last))
+	      {
+		expr = last;
+		last = TREE_CHAIN (last);
+	      }
+
+	    if (TREE_CODE (last) == SCOPE_STMT
+		&& TREE_CODE (expr) == EXPR_STMT)
+	      TREE_ADDRESSABLE (expr) = 1;
+	  }
+
 	expand_stmt (STMT_EXPR_STMT (exp));
 	expand_end_stmt_expr (rtl_expr);
 	result = expand_expr (rtl_expr, target, tmode, modifier);
Index: gcc/c-common.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/c-common.h,v
retrieving revision 1.103
diff -u -p -r1.103 c-common.h
--- gcc/c-common.h 2001/12/04 22:55:36 1.103
+++ gcc/c-common.h 2001/12/05 17:00:18
@@ -708,6 +708,7 @@ extern void add_c_tree_codes		        PA
 extern void genrtl_do_pushlevel                 PARAMS ((void));
 extern void genrtl_goto_stmt                    PARAMS ((tree));
 extern void genrtl_expr_stmt                    PARAMS ((tree));
+extern void genrtl_expr_stmt_value              PARAMS ((tree, int));
 extern void genrtl_decl_stmt                    PARAMS ((tree));
 extern void genrtl_if_stmt                      PARAMS ((tree));
 extern void genrtl_while_stmt                   PARAMS ((tree));
Index: gcc/c-semantics.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/c-semantics.c,v
retrieving revision 1.34
diff -u -p -r1.34 c-semantics.c
--- gcc/c-semantics.c 2001/10/11 03:15:23 1.34
+++ gcc/c-semantics.c 2001/12/05 17:00:18
@@ -305,12 +305,27 @@ genrtl_goto_stmt (destination)
     expand_computed_goto (destination);
 }
 
-/* Generate the RTL for EXPR, which is an EXPR_STMT.  */
+/* Generate the RTL for EXPR, which is an EXPR_STMT.  Provided just
+   for backward compatibility.  genrtl_expr_stmt_value() should be
+   used for new code.  */
 
-void 
+void
 genrtl_expr_stmt (expr)
      tree expr;
 {
+  genrtl_expr_stmt_value (expr, -1);
+}
+
+/* Generate the RTL for EXPR, which is an EXPR_STMT.  WANT_VALUE tells
+   whether to (1) save the value of the expression, (0) discard it or
+   (-1) use expr_stmts_for_value to tell.  The use of -1 is
+   deprecated, and retained only for backward compatibility.  */
+
+void 
+genrtl_expr_stmt_value (expr, want_value)
+     tree expr;
+     int want_value;
+{
   if (expr != NULL_TREE)
     {
       emit_line_note (input_filename, lineno);
@@ -319,7 +334,7 @@ genrtl_expr_stmt (expr)
 	expand_start_target_temps ();
       
       if (expr != error_mark_node)
-	expand_expr_stmt (expr);
+	expand_expr_stmt_value (expr, want_value);
       
       if (stmts_are_full_exprs_p ())
 	expand_end_target_temps ();
@@ -748,7 +763,7 @@ expand_stmt (t)
 	  break;
 
 	case EXPR_STMT:
-	  genrtl_expr_stmt (EXPR_STMT_EXPR (t));
+	  genrtl_expr_stmt_value (EXPR_STMT_EXPR (t), TREE_ADDRESSABLE (t));
 	  break;
 
 	case DECL_STMT:
Index: gcc/expr.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/expr.c,v
retrieving revision 1.392
diff -u -p -r1.392 expr.c
--- gcc/expr.c 2001/12/05 14:15:32 1.392
+++ gcc/expr.c 2001/12/05 17:00:20
@@ -6662,7 +6662,7 @@ expand_expr (exp, target, tmode, modifie
 
     case LABELED_BLOCK_EXPR:
       if (LABELED_BLOCK_BODY (exp))
-	expand_expr_stmt (LABELED_BLOCK_BODY (exp));
+	expand_expr_stmt_value (LABELED_BLOCK_BODY (exp), 0);
       /* Should perhaps use expand_label, but this is simpler and safer.  */
       do_pending_stack_adjust ();
       emit_label (label_rtx (LABELED_BLOCK_LABEL (exp)));
@@ -6677,7 +6677,7 @@ expand_expr (exp, target, tmode, modifie
     case LOOP_EXPR:
       push_temp_slots ();
       expand_start_loop (1);
-      expand_expr_stmt (TREE_OPERAND (exp, 0));
+      expand_expr_stmt_value (TREE_OPERAND (exp, 0), 0);
       expand_end_loop ();
       pop_temp_slots ();
 
Index: gcc/stmt.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/stmt.c,v
retrieving revision 1.235
diff -u -p -r1.235 stmt.c
--- gcc/stmt.c 2001/12/04 17:37:23 1.235
+++ gcc/stmt.c 2001/12/05 17:00:21
@@ -2143,16 +2143,37 @@ resolve_operand_name_1 (p, outputs, inpu
 }
 
 /* Generate RTL to evaluate the expression EXP
-   and remember it in case this is the VALUE in a ({... VALUE; }) constr.  */
+   and remember it in case this is the VALUE in a ({... VALUE; }) constr.
+   Provided just for backward-compatibility.  expand_expr_stmt_value()
+   should be used for new code.  */
 
 void
 expand_expr_stmt (exp)
      tree exp;
 {
+  expand_expr_stmt_value (exp, -1);
+}
+
+/* Generate RTL to evaluate the expression EXP.  WANT_VALUE tells
+   whether to (1) save the value of the expression, (0) discard it or
+   (-1) use expr_stmts_for_value to tell.  The use of -1 is
+   deprecated, and retained only for backward compatibility.  */
+
+void
+expand_expr_stmt_value (exp, want_value)
+     tree exp;
+     int want_value;
+{
+  rtx value;
+  tree type;
+
+  if (want_value == -1)
+    want_value = expr_stmts_for_value != 0;
+
   /* 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.  */
-  if (expr_stmts_for_value == 0 && exp != error_mark_node)
+  if (! want_value && exp != error_mark_node)
     {
       if (! TREE_SIDE_EFFECTS (exp))
 	{
@@ -2168,34 +2189,31 @@ expand_expr_stmt (exp)
 
   /* If EXP is of function type and we are expanding statements for
      value, convert it to pointer-to-function.  */
-  if (expr_stmts_for_value && TREE_CODE (TREE_TYPE (exp)) == FUNCTION_TYPE)
+  if (want_value && TREE_CODE (TREE_TYPE (exp)) == FUNCTION_TYPE)
     exp = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (exp)), exp);
 
   /* The call to `expand_expr' could cause last_expr_type and
      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
-				  ? NULL_RTX : const0_rtx),
-				 VOIDmode, 0);
-  last_expr_type = TREE_TYPE (exp);
+  value = expand_expr (exp, want_value ? NULL_RTX : const0_rtx,
+		       VOIDmode, 0);
+  type = TREE_TYPE (exp);
 
   /* If all we do is reference a volatile value in memory,
      copy it to a register to be sure it is actually touched.  */
-  if (last_expr_value != 0 && GET_CODE (last_expr_value) == MEM
-      && TREE_THIS_VOLATILE (exp))
+  if (value && GET_CODE (value) == MEM && TREE_THIS_VOLATILE (exp))
     {
-      if (TYPE_MODE (TREE_TYPE (exp)) == VOIDmode)
+      if (TYPE_MODE (type) == VOIDmode)
 	;
-      else if (TYPE_MODE (TREE_TYPE (exp)) != BLKmode)
-	copy_to_reg (last_expr_value);
+      else if (TYPE_MODE (type) != BLKmode)
+	value = copy_to_reg (value);
       else
 	{
 	  rtx lab = gen_label_rtx ();
 
 	  /* Compare the value with itself to reference it.  */
-	  emit_cmp_and_jump_insns (last_expr_value, last_expr_value, EQ,
-				   expand_expr (TYPE_SIZE (last_expr_type),
+	  emit_cmp_and_jump_insns (value, value, EQ,
+				   expand_expr (TYPE_SIZE (type),
 						NULL_RTX, VOIDmode, 0),
 				   BLKmode, 0, lab);
 	  emit_label (lab);
@@ -2204,13 +2222,19 @@ expand_expr_stmt (exp)
 
   /* If this expression is part of a ({...}) and is in memory, we may have
      to preserve temporaries.  */
-  preserve_temp_slots (last_expr_value);
+  preserve_temp_slots (value);
 
   /* Free any temporaries used to evaluate this expression.  Any temporary
      used as a result of this expression will already have been preserved
      above.  */
   free_temp_slots ();
 
+  if (want_value)
+    {
+      last_expr_value = value;
+      last_expr_type = type;
+    }
+
   emit_queue ();
 }
 
@@ -2347,6 +2371,7 @@ expand_start_stmt_expr ()
   start_sequence_for_rtl_expr (t);
   NO_DEFER_POP;
   expr_stmts_for_value++;
+  last_expr_value = NULL_RTX;
   return t;
 }
 
@@ -2368,15 +2393,11 @@ expand_end_stmt_expr (t)
 {
   OK_DEFER_POP;
 
-  if (last_expr_type == 0)
+  if (! last_expr_value || ! last_expr_type)
     {
-      last_expr_type = void_type_node;
       last_expr_value = const0_rtx;
+      last_expr_type = void_type_node;
     }
-  else if (last_expr_value == 0)
-    /* There are some cases where this can happen, such as when the
-       statement is void type.  */
-    last_expr_value = const0_rtx;
   else if (GET_CODE (last_expr_value) != REG && ! CONSTANT_P (last_expr_value))
     /* Remove any possible QUEUED.  */
     last_expr_value = protect_from_queue (last_expr_value, 0);
Index: gcc/tree-inline.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/tree-inline.c,v
retrieving revision 1.11
diff -u -p -r1.11 tree-inline.c
--- gcc/tree-inline.c 2001/12/04 10:34:40 1.11
+++ gcc/tree-inline.c 2001/12/05 17:00:21
@@ -624,7 +624,9 @@ declare_return_variable (id, use_stmt)
     *use_stmt = build_stmt (EXPR_STMT,
 			    build1 (NOP_EXPR, TREE_TYPE (TREE_TYPE (fn)),
 				    var));
-      
+
+  TREE_ADDRESSABLE (*use_stmt) = 1;
+
   /* Build the declaration statement if FN does not return an
      aggregate.  */
   if (need_return_decl)
Index: gcc/tree.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/tree.h,v
retrieving revision 1.292
diff -u -p -r1.292 tree.h
--- gcc/tree.h 2001/12/04 15:10:04 1.292
+++ gcc/tree.h 2001/12/05 17:00:21
@@ -161,7 +161,9 @@ struct tree_common
 
        TREE_ADDRESSABLE in
    	   VAR_DECL, FUNCTION_DECL, FIELD_DECL, CONSTRUCTOR, LABEL_DECL,
-	   ..._TYPE, IDENTIFIER_NODE
+	   ..._TYPE, IDENTIFIER_NODE.
+	   In a STMT_EXPR, it means we want the result of the enclosed
+	   expression.
 
    static_flag:
 
@@ -258,8 +260,7 @@ struct tree_common
 	   expressions, VAR_DECL, PARM_DECL, FIELD_DECL, FUNCTION_DECL,
 	   IDENTIFIER_NODE
        TYPE_BOUNDED in
-	   ..._TYPE
-*/
+	   ..._TYPE */
 
 /* Define accessors for the fields that all tree nodes have
    (though some fields are not used for all kinds of nodes).  */
@@ -2709,6 +2710,7 @@ extern void expand_fixups			PARAMS ((rtx
 extern tree expand_start_stmt_expr		PARAMS ((void));
 extern tree expand_end_stmt_expr		PARAMS ((tree));
 extern void expand_expr_stmt			PARAMS ((tree));
+extern void expand_expr_stmt_value		PARAMS ((tree, int));
 extern int warn_if_unused_value			PARAMS ((tree));
 extern void expand_decl_init			PARAMS ((tree));
 extern void clear_last_expr			PARAMS ((void));


-- 
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]