C PATCH to kill c_save_expr or towards delayed folding for the C FE

Marek Polacek polacek@redhat.com
Fri May 12 19:48:00 GMT 2017


[ This patch depends on my previous patch that removes the fold call from
  save_expr. ]

In the effort of reducing early folding, we should avoid calling c_fully_fold
blithely, except when needed for e.g. initializers.  This is a teeny tiny step
down that path.  This patch does away with c_save_expr, which always fully
folded the expression.  Instead, I moved the SAVE_EXPR folding to c_fully_fold.
The new flag SAVE_EXPR_FOLDED_P ensures we only fold the contents once.  Also,
if the contents of the SAVE_EXPR are tree_invariant_p, we don't have to wrap
the expression with SAVE_EXPR.  I wonder if I should instead introduce a hash
map, much like C++; if we fold the contents of a SAVE_EXPR to an invariant, I
think we'll fold the expression multiple times, because in that case we cannot
check the SAVE_EXPR_FOLDED_P flag.

Comments/future directions very much appreciated.

Bootstrapped/regtested on x86_64-linux, ok for trunk once the other patch makes
it in?

2017-05-12  Marek Polacek  <polacek@redhat.com>

	* c-common.c (c_save_expr): Remove.
	(c_common_truthvalue_conversion): Remove a call to c_save_expr.
	* c-common.h (c_save_expr): Remove declaration.

	* c-convert.c (convert): Replace c_save_expr with save_expr.  Don't
	call c_fully_fold.
	* c-decl.c (grokdeclarator): Replace c_save_expr with save_expr. 
	* c-fold.c (c_fully_fold_internal): Handle SAVE_EXPR.
	* c-parser.c (c_parser_declaration_or_fndef): Replace c_save_expr with
	save_expr.
	(c_parser_conditional_expression): Likewise.
	* c-tree.h (SAVE_EXPR_FOLDED_P): Define.
	* c-typeck.c (build_modify_expr): Replace c_save_expr with save_expr.
	(process_init_element): Likewise.
	(build_binary_op): Likewise.
	(handle_omp_array_sections_1): Likewise.

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index ad686d2..f606e94 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -3164,24 +3164,6 @@ c_wrap_maybe_const (tree expr, bool non_const)
   return expr;
 }
 
-/* Wrap a SAVE_EXPR around EXPR, if appropriate.  Like save_expr, but
-   for C folds the inside expression and wraps a C_MAYBE_CONST_EXPR
-   around the SAVE_EXPR if needed so that c_fully_fold does not need
-   to look inside SAVE_EXPRs.  */
-
-tree
-c_save_expr (tree expr)
-{
-  bool maybe_const = true;
-  if (c_dialect_cxx ())
-    return save_expr (expr);
-  expr = c_fully_fold (expr, false, &maybe_const);
-  expr = save_expr (expr);
-  if (!maybe_const)
-    expr = c_wrap_maybe_const (expr, true);
-  return expr;
-}
-
 /* Return whether EXPR is a declaration whose address can never be
    NULL.  */
 
@@ -3436,7 +3418,7 @@ c_common_truthvalue_conversion (location_t location, tree expr)
 
   if (TREE_CODE (TREE_TYPE (expr)) == COMPLEX_TYPE)
     {
-      tree t = (in_late_binary_op ? save_expr (expr) : c_save_expr (expr));
+      tree t = save_expr (expr);
       expr = (build_binary_op
 	      (EXPR_LOCATION (expr),
 	       (TREE_SIDE_EFFECTS (expr)
diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
index 9e3982d..3981544 100644
--- gcc/c-family/c-common.h
+++ gcc/c-family/c-common.h
@@ -836,7 +836,6 @@ extern enum conversion_safety unsafe_conversion_p (location_t, tree, tree,
 extern bool decl_with_nonnull_addr_p (const_tree);
 extern tree c_fully_fold (tree, bool, bool *);
 extern tree c_wrap_maybe_const (tree, bool);
-extern tree c_save_expr (tree);
 extern tree c_common_truthvalue_conversion (location_t, tree);
 extern void c_apply_type_quals_to_decl (int, tree);
 extern tree c_sizeof_or_alignof_type (location_t, tree, bool, bool, int);
diff --git gcc/c/c-convert.c gcc/c/c-convert.c
index 163feff..a7764f0 100644
--- gcc/c/c-convert.c
+++ gcc/c/c-convert.c
@@ -111,13 +111,7 @@ convert (tree type, tree expr)
 	  && COMPLETE_TYPE_P (type)
 	  && do_ubsan_in_current_function ())
 	{
-	  if (in_late_binary_op)
-	    expr = save_expr (expr);
-	  else
-	    {
-	      expr = c_save_expr (expr);
-	      expr = c_fully_fold (expr, false, NULL);
-	    }
+	  expr = save_expr (expr);
 	  tree check = ubsan_instrument_float_cast (loc, type, expr);
 	  expr = fold_build1 (FIX_TRUNC_EXPR, type, expr);
 	  if (check == NULL_TREE)
@@ -146,8 +140,7 @@ convert (tree type, tree expr)
 
     case COMPLEX_TYPE:
       /* If converting from COMPLEX_TYPE to a different COMPLEX_TYPE
-	 and e is not COMPLEX_EXPR, convert_to_complex uses save_expr,
-	 but for the C FE c_save_expr needs to be called instead.  */
+	 and E is not COMPLEX_EXPR, convert_to_complex uses save_expr.  */
       if (TREE_CODE (TREE_TYPE (e)) == COMPLEX_TYPE)
 	{
 	  if (TREE_CODE (e) != COMPLEX_EXPR)
@@ -155,10 +148,7 @@ convert (tree type, tree expr)
 	      tree subtype = TREE_TYPE (type);
 	      tree elt_type = TREE_TYPE (TREE_TYPE (e));
 
-	      if (in_late_binary_op)
-		e = save_expr (e);
-	      else
-		e = c_save_expr (e);
+	      e = save_expr (e);
 	      ret
 		= fold_build2_loc (loc, COMPLEX_EXPR, type,
 				   convert (subtype,
diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index b779d37..8ae09c4 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -6076,7 +6076,7 @@ grokdeclarator (const struct c_declarator *declarator,
 			&& do_ubsan_in_current_function ())
 		      {
 			/* Evaluate the array size only once.  */
-			size = c_save_expr (size);
+			size = save_expr (size);
 			size = c_fully_fold (size, false, NULL);
 		        size = fold_build2 (COMPOUND_EXPR, TREE_TYPE (size),
 					    ubsan_instrument_vla (loc, size),
diff --git gcc/c/c-fold.c gcc/c/c-fold.c
index b060d76..1baee44 100644
--- gcc/c/c-fold.c
+++ gcc/c/c-fold.c
@@ -120,12 +120,11 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands,
   bool unused_p;
   source_range old_range;
 
-  /* Constants, declarations, statements, errors, SAVE_EXPRs and
-     anything else not counted as an expression cannot usefully be
-     folded further at this point.  */
+  /* Constants, declarations, statements, errors, and anything else not
+     counted as an expression cannot usefully be folded further at this
+     point.  */
   if (!IS_EXPR_CODE_CLASS (kind)
-      || kind == tcc_statement
-      || code == SAVE_EXPR)
+      || kind == tcc_statement)
     return expr;
 
   if (IS_EXPR_CODE_CLASS (kind))
@@ -565,6 +564,25 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands,
 	 appropriate in any particular case.  */
       gcc_unreachable ();
 
+    case SAVE_EXPR:
+      /* Make sure to fold the contents of a SAVE_EXPR exactly once.  */
+      if (!SAVE_EXPR_FOLDED_P (expr))
+	{
+	  op0 = TREE_OPERAND (expr, 0);
+	  op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
+				       maybe_const_itself, for_int_const);
+	  /* Don't wrap the folded tree in a SAVE_EXPR if we don't
+	     have to.  */
+	  if (tree_invariant_p (op0))
+	    ret = op0;
+	  else
+	    {
+	      TREE_OPERAND (expr, 0) = op0;
+	      SAVE_EXPR_FOLDED_P (expr) = true;
+	    }
+	}
+      goto out;
+
     default:
       /* Various codes may appear through folding built-in functions
 	 and their arguments.  */
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 90d2d17..96c0749 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -1880,7 +1880,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
 		  bool vm_type = variably_modified_type_p (init_type,
 							   NULL_TREE);
 		  if (vm_type)
-		    init.value = c_save_expr (init.value);
+		    init.value = save_expr (init.value);
 		  finish_init ();
 		  specs->typespec_kind = ctsk_typeof;
 		  specs->locations[cdw_typedef] = init_loc;
@@ -6500,7 +6500,7 @@ c_parser_conditional_expression (c_parser *parser, struct c_expr *after,
 	e = TREE_OPERAND (e, 1);
       warn_for_omitted_condop (middle_loc, e);
       /* Make sure first operand is calculated only once.  */
-      exp1.value = c_save_expr (default_conversion (cond.value));
+      exp1.value = save_expr (default_conversion (cond.value));
       if (eptype)
 	exp1.value = build1 (EXCESS_PRECISION_EXPR, eptype, exp1.value);
       exp1.original_type = NULL;
diff --git gcc/c/c-tree.h gcc/c/c-tree.h
index 8d232a4..a27a2c7 100644
--- gcc/c/c-tree.h
+++ gcc/c/c-tree.h
@@ -113,6 +113,10 @@ along with GCC; see the file COPYING3.  If not see
    subexpression meaning it is not a constant expression.  */
 #define CONSTRUCTOR_NON_CONST(EXPR) TREE_LANG_FLAG_1 (CONSTRUCTOR_CHECK (EXPR))
 
+/* For a SAVE_EXPR, nonzero if the contents of the SAVE_EXPR have already
+   been folded.  */
+#define SAVE_EXPR_FOLDED_P(EXP)	TREE_LANG_FLAG_1 (SAVE_EXPR_CHECK (EXP))
+
 /* Record parser information about an expression that is irrelevant
    for code generation alongside a tree representing its value.  */
 struct c_expr
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 6f9909c..1edf521 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -5777,7 +5777,7 @@ build_modify_expr (location_t location, tree lhs, tree lhs_origtype,
 	     that modify LHS.  */
 	  if (TREE_SIDE_EFFECTS (rhs))
 	    {
-	      newrhs = in_late_binary_op ? save_expr (rhs) : c_save_expr (rhs);
+	      newrhs = save_expr (rhs);
 	      rhseval = newrhs;
 	    }
 	  newrhs = build_binary_op (location,
@@ -9301,7 +9301,7 @@ process_init_element (location_t loc, struct c_expr value, bool implicit,
 	      semantic_type = TREE_TYPE (value.value);
 	      value.value = TREE_OPERAND (value.value, 0);
 	    }
-	  value.value = c_save_expr (value.value);
+	  value.value = save_expr (value.value);
 	  if (semantic_type)
 	    value.value = build1 (EXCESS_PRECISION_EXPR, semantic_type,
 				  value.value);
@@ -11621,7 +11621,7 @@ build_binary_op (location_t location, enum tree_code code,
 	    return error_mark_node;
 	  if (first_complex)
 	    {
-	      op0 = c_save_expr (op0);
+	      op0 = save_expr (op0);
 	      real = build_unary_op (EXPR_LOCATION (orig_op0), REALPART_EXPR,
 				     op0, true);
 	      imag = build_unary_op (EXPR_LOCATION (orig_op0), IMAGPART_EXPR,
@@ -11630,7 +11630,7 @@ build_binary_op (location_t location, enum tree_code code,
 		{
 		case MULT_EXPR:
 		case TRUNC_DIV_EXPR:
-		  op1 = c_save_expr (op1);
+		  op1 = save_expr (op1);
 		  imag = build2 (resultcode, real_type, imag, op1);
 		  /* Fall through.  */
 		case PLUS_EXPR:
@@ -11643,7 +11643,7 @@ build_binary_op (location_t location, enum tree_code code,
 	    }
 	  else
 	    {
-	      op1 = c_save_expr (op1);
+	      op1 = save_expr (op1);
 	      real = build_unary_op (EXPR_LOCATION (orig_op1), REALPART_EXPR,
 				     op1, true);
 	      imag = build_unary_op (EXPR_LOCATION (orig_op1), IMAGPART_EXPR,
@@ -11651,7 +11651,7 @@ build_binary_op (location_t location, enum tree_code code,
 	      switch (code)
 		{
 		case MULT_EXPR:
-		  op0 = c_save_expr (op0);
+		  op0 = save_expr (op0);
 		  imag = build2 (resultcode, real_type, op0, imag);
 		  /* Fall through.  */
 		case PLUS_EXPR:
@@ -11835,8 +11835,8 @@ build_binary_op (location_t location, enum tree_code code,
       && !require_constant_value)
     {
       /* OP0 and/or OP1 might have side-effects.  */
-      op0 = c_save_expr (op0);
-      op1 = c_save_expr (op1);
+      op0 = save_expr (op0);
+      op1 = save_expr (op1);
       op0 = c_fully_fold (op0, false, NULL);
       op1 = c_fully_fold (op1, false, NULL);
       if (doing_div_or_mod && (flag_sanitize & (SANITIZE_DIVIDE
@@ -12435,7 +12435,7 @@ handle_omp_array_sections_1 (tree c, tree t, vec<tree> &types,
       /* For [lb:] we will need to evaluate lb more than once.  */
       if (length == NULL_TREE && OMP_CLAUSE_CODE (c) != OMP_CLAUSE_DEPEND)
 	{
-	  tree lb = c_save_expr (low_bound);
+	  tree lb = save_expr (low_bound);
 	  if (lb != low_bound)
 	    {
 	      TREE_PURPOSE (t) = lb;
@@ -12480,7 +12480,7 @@ handle_omp_array_sections_1 (tree c, tree t, vec<tree> &types,
   if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_DEPEND)
     types.safe_push (TREE_TYPE (ret));
   /* We will need to evaluate lb more than once.  */
-  tree lb = c_save_expr (low_bound);
+  tree lb = save_expr (low_bound);
   if (lb != low_bound)
     {
       TREE_PURPOSE (t) = lb;

	Marek



More information about the Gcc-patches mailing list