Re: [PATCH] Add save_expr langhook (PR c/68513)

[Richard Biener <rguenther at suse dot de>]

On November 27, 2015 7:55:43 PM GMT+01:00, Marek Polacek <polacek@redhat.com> wrote:
>As suggested here
><https://gcc.gnu.org/ml/gcc-patches/2015-11/msg03271.html>
>and here <https://gcc.gnu.org/ml/gcc-patches/2015-11/msg03282.html>,
>this patch
>adds a new langhook to distinguish whether to call c_save_expr or
>save_expr
>from match.pd.  Does this look reasonable?
>
>I didn't know where to put setting of in_late_processing.  With the
>current
>placement, we won't (for valid programs) call c_save_expr from
>c_genericize
>or c_gimplify_expr.
>
>I suppose I should also modify save_expr in fold-const.c to call it via
>the
>langhook, if this approach is sane.  Dunno.

I don't like this at all.

Different approach: after the FE folds (unexpectedly?), scan the result for SAVE_EXPRs and if found, drop the folding.

Richard.

>Bootstrapped/regtested on x86_64-linux.
>
>2015-11-27  Marek Polacek  <polacek@redhat.com>
>
>	PR c/68513
>	* c-common.c (in_late_processing): New global.
>	(c_common_save_expr): New function.
>	* c-common.h (in_late_processing, c_common_save_expr): Declare.
>
>	* c-objc-common.h (LANG_HOOKS_SAVE_EXPR): Define.
>	* c-parser.c (c_parser_compound_statement): Set IN_LATE_PROCESSING.
>
>	* generic-match-head.c: Include "langhooks.h".
>	* genmatch.c (dt_simplify::gen_1): Call save_expr via langhook.
>	* langhooks-def.h (LANG_HOOKS_SAVE_EXPR): Define.
>	* langhooks.h (struct lang_hooks): Add save_expr langhook.
>
>	* gcc.dg/torture/pr68513.c: New test.
>
>diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
>index fe0a235..850bee9 100644
>--- gcc/c-family/c-common.c
>+++ gcc/c-family/c-common.c
>@@ -271,6 +271,12 @@ int c_inhibit_evaluation_warnings;
>    be generated.  */
> bool in_late_binary_op;
> 
>+/* When true, all the constant expression checks from parsing should
>have been
>+   done.  This is used so that fold knows whether to call c_save_expr
>(thus
>+   c_fully_fold is called on the expression), or whether to call
>save_expr via
>+   c_common_save_expr langhook.  */
>+bool in_late_processing;
>+
> /* Whether lexing has been completed, so subsequent preprocessor
>    errors should use the compiler's input_location.  */
> bool done_lexing = false;
>@@ -4928,6 +4934,15 @@ c_save_expr (tree expr)
>   return expr;
> }
> 
>+/* The C version of the save_expr langhook.  Either call save_expr or
>c_save_expr,
>+   depending on IN_LATE_PROCESSING.  */
>+
>+tree
>+c_common_save_expr (tree expr)
>+{
>+  return in_late_processing ? save_expr (expr) : c_save_expr (expr);
>+}
>+
> /* Return whether EXPR is a declaration whose address can never be
>    NULL.  */
> 
>diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
>index bad8d05..e2d4ba9 100644
>--- gcc/c-family/c-common.h
>+++ gcc/c-family/c-common.h
>@@ -771,6 +771,7 @@ extern void c_register_addr_space (const char *str,
>addr_space_t as);
> 
> /* In c-common.c.  */
> extern bool in_late_binary_op;
>+extern bool in_late_processing;
> extern const char *c_addr_space_name (addr_space_t as);
> extern tree identifier_global_value (tree);
> extern tree c_linkage_bindings (tree);
>@@ -812,6 +813,7 @@ extern tree c_fully_fold (tree, bool, bool *);
> extern tree decl_constant_value_for_optimization (tree);
> extern tree c_wrap_maybe_const (tree, bool);
> extern tree c_save_expr (tree);
>+extern tree c_common_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-objc-common.h gcc/c/c-objc-common.h
>index 50c9f54..9fd3722 100644
>--- gcc/c/c-objc-common.h
>+++ gcc/c/c-objc-common.h
>@@ -60,6 +60,8 @@ along with GCC; see the file COPYING3.  If not see
> #define LANG_HOOKS_BUILTIN_FUNCTION c_builtin_function
> #undef  LANG_HOOKS_BUILTIN_FUNCTION_EXT_SCOPE
>#define LANG_HOOKS_BUILTIN_FUNCTION_EXT_SCOPE
>c_builtin_function_ext_scope
>+#undef LANG_HOOKS_SAVE_EXPR
>+#define LANG_HOOKS_SAVE_EXPR c_common_save_expr
> 
> /* Attribute hooks.  */
> #undef LANG_HOOKS_COMMON_ATTRIBUTE_TABLE
>diff --git gcc/c/c-parser.c gcc/c/c-parser.c
>index 0259f66..3f7c458 100644
>--- gcc/c/c-parser.c
>+++ gcc/c/c-parser.c
>@@ -4586,6 +4586,9 @@ c_parser_compound_statement (c_parser *parser)
> {
>   tree stmt;
>   location_t brace_loc;
>+
>+  in_late_processing = false;
>+
>   brace_loc = c_parser_peek_token (parser)->location;
>   if (!c_parser_require (parser, CPP_OPEN_BRACE, "expected %<{%>"))
>     {
>@@ -4598,6 +4601,9 @@ c_parser_compound_statement (c_parser *parser)
>   stmt = c_begin_compound_stmt (true);
>   c_parser_compound_statement_nostart (parser);
> 
>+  /* From now on, the fold machinery shouldn't call c_save_expr.  */
>+  in_late_processing = true;
>+
>/* If the compound stmt contains array notations, then we expand them. 
>*/
>   if (flag_cilkplus && contains_array_notation_expr (stmt))
>     stmt = expand_array_notation_exprs (stmt);
>diff --git gcc/generic-match-head.c gcc/generic-match-head.c
>index f55f91e..8fc9d40 100644
>--- gcc/generic-match-head.c
>+++ gcc/generic-match-head.c
>@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
> #include "builtins.h"
> #include "dumpfile.h"
> #include "case-cfn-macros.h"
>+#include "langhooks.h"
> 
> 
> /* Routine to determine if the types T1 and T2 are effectively
>diff --git gcc/genmatch.c gcc/genmatch.c
>index 67d1c66..c73a94d 100644
>--- gcc/genmatch.c
>+++ gcc/genmatch.c
>@@ -3114,7 +3114,8 @@ dt_simplify::gen_1 (FILE *f, int indent, bool
>gimple, operand *result)
> 		  continue;
> 		if (cinfo.info[i].result_use_count > 1)
> 		  fprintf_indent (f, indent,
>-				  "captures[%d] = save_expr (captures[%d]);\n",
>+				  "captures[%d] = lang_hooks.save_expr "
>+				  "(captures[%d]);\n",
> 				  i, i);
> 	      }
> 	  for (unsigned j = 0; j < e->ops.length (); ++j)
>diff --git gcc/langhooks-def.h gcc/langhooks-def.h
>index 18ac84d..6f5babe 100644
>--- gcc/langhooks-def.h
>+++ gcc/langhooks-def.h
>@@ -120,6 +120,7 @@ extern bool lhd_omp_mappable_type (tree);
> #define LANG_HOOKS_BLOCK_MAY_FALLTHRU	hook_bool_const_tree_true
> #define LANG_HOOKS_EH_USE_CXA_END_CLEANUP	false
> #define LANG_HOOKS_DEEP_UNSHARING	false
>+#define LANG_HOOKS_SAVE_EXPR		save_expr
> 
> /* Attribute hooks.  */
> #define LANG_HOOKS_ATTRIBUTE_TABLE		NULL
>@@ -313,7 +314,8 @@ extern void lhd_end_section (void);
>   LANG_HOOKS_EH_PROTECT_CLEANUP_ACTIONS, \
>   LANG_HOOKS_BLOCK_MAY_FALLTHRU, \
>   LANG_HOOKS_EH_USE_CXA_END_CLEANUP, \
>-  LANG_HOOKS_DEEP_UNSHARING \
>+  LANG_HOOKS_DEEP_UNSHARING, \
>+  LANG_HOOKS_SAVE_EXPR \
> }
> 
> #endif /* GCC_LANG_HOOKS_DEF_H */
>diff --git gcc/langhooks.h gcc/langhooks.h
>index d8d01fa..18f332d 100644
>--- gcc/langhooks.h
>+++ gcc/langhooks.h
>@@ -488,6 +488,9 @@ struct lang_hooks
>      gimplification.  */
>   bool deep_unsharing;
> 
>+  /* Used by the fold machinery to either call save_expr or
>c_save_expr.  */
>+  tree (*save_expr) (tree);
>+
> /* Whenever you add entries here, make sure you adjust langhooks-def.h
>      and langhooks.c accordingly.  */
> };
>diff --git gcc/testsuite/gcc.dg/torture/pr68513.c
>gcc/testsuite/gcc.dg/torture/pr68513.c
>index e69de29..4e08b29 100644
>--- gcc/testsuite/gcc.dg/torture/pr68513.c
>+++ gcc/testsuite/gcc.dg/torture/pr68513.c
>@@ -0,0 +1,13 @@
>+/* PR c/68513 */
>+/* { dg-do compile } */
>+
>+int i;
>+unsigned u;
>+volatile unsigned int *e;
>+
>+void
>+fn1 (void)
>+{
>+  (short) ((i ? *e : 0) & ~u | i & u);
>+  (short) (((0, 0) ? *e : 0) & ~u | i & u);
>+}
>
>	Marek