Patchwork [C++11,4.9] Add missing REDUC_PLUS_EXPR case to potential_constant_expression_1.

login
register
mail settings
Submitter James Greenhalgh
Date April 19, 2013, 9:39 a.m.
Message ID <1366364388-14979-1-git-send-email-james.greenhalgh@arm.com>
Download mbox | patch
Permalink /patch/237894/
State New
Headers show

Comments

James Greenhalgh - April 19, 2013, 9:39 a.m.
Hi,

> I still think getting rid of TARGET_FOLD_BUILTIN and replacing it with
> TARGET_FOLD_STMT that only operates on GIMPLE is the way to go.

Would that look something like the attached? (The patch isn't
polished, but hacks enough together to function).

In these patches we add TARGET_FOLD_STMT and an implimentation in
aarch64_fold_stmt. TARGET_FOLD_STMT takes a gimple_stmt_iterator
and returns whether any change was made to the gimple stream.

Then TARGET_FOLD_STMT is allowed to fold to anything which is
valid GIMPLE, and TARGET_FOLD_BUILTIN is required to fold to
something in the intersection of GIMPLE and GENERIC.

I don't especially like passing a gimple_stmt_iterator as it means
exposing all sorts of extra gunk to the backend. Without it, the
interface is not as flexible. This is not neccessarily a problem, but
giving the backend the full gimple_stmt_iterator seems to me to be
more useful.

If this was what you were asking for I can clean the patch up
and give it proper testing. If not, I would appreciate a prod
in the correct direction.

Thanks,

James Greenhalgh
Graduate Engineer
ARM

---
gcc/

2013-04-19  James Greenhalgh  <james.greenhalgh@arm.com>

	* coretypes.h (gimple_stmt_iterator_d): Forward declare.
	(gimple_stmt_iterator): New typedef.
	* doc/tm.texi.in (TARGET_FOLD_STMT): New.
	* gimple-fold.c (fold_stmt_1): Call target hook fold_stmt.
	* gimple.h (gimple_stmt_iterator): Rename to...
	(gimple_stmt_iterator_d): ... This.
	* hooks.c (hook_bool_gsiptr_bool_false): New.
	* target.def (fold_stmt): New.
	* doc/tm.texi: Regenerate.

2013-04-19  James Greenhalgh  <james.greenhalgh@arm.com>

	* config/aarch64/aarch64-builtins.c
	(aarch64_fold_builtin): Move folds to gimple-specific tree codes
	to aarch64_fold_stmt.
	(aarch64_fold_stmt): New.
	* config/aarch64/aarch64-protos.h (aarch64_fold_stmt): New.
	* config/aarch64/aarch64.c (TARGET_FOLD_STMT): Define.
Richard Guenther - April 19, 2013, 10:13 a.m.
On Fri, Apr 19, 2013 at 11:39 AM, James Greenhalgh
<james.greenhalgh@arm.com> wrote:
> Hi,
>
>> I still think getting rid of TARGET_FOLD_BUILTIN and replacing it with
>> TARGET_FOLD_STMT that only operates on GIMPLE is the way to go.
>
> Would that look something like the attached? (The patch isn't
> polished, but hacks enough together to function).
>
> In these patches we add TARGET_FOLD_STMT and an implimentation in
> aarch64_fold_stmt. TARGET_FOLD_STMT takes a gimple_stmt_iterator
> and returns whether any change was made to the gimple stream.
>
> Then TARGET_FOLD_STMT is allowed to fold to anything which is
> valid GIMPLE, and TARGET_FOLD_BUILTIN is required to fold to
> something in the intersection of GIMPLE and GENERIC.
>
> I don't especially like passing a gimple_stmt_iterator as it means
> exposing all sorts of extra gunk to the backend. Without it, the
> interface is not as flexible. This is not neccessarily a problem, but
> giving the backend the full gimple_stmt_iterator seems to me to be
> more useful.
>
> If this was what you were asking for I can clean the patch up
> and give it proper testing. If not, I would appreciate a prod
> in the correct direction.

Yes, it looks basically ok.  I'd probably restrict it to folding target builtins
though - similar to what TARGET_FOLD_BUILTIN did.  Exactly to not
expose all stmts to the backends.  That is, move the target hook call
to gimple_fold_call, after the inplace check (and remove the inplace
argument of the target hook), and call it only for DECL_BUILT_IN_CLASS
BUILT_IN_MD.

Not sure if TARGET_FOLD_STMT is then still an appropriate name ...
TARGET_FOLD_BUILTIN is already taken unfortunately.  Maybe
TARGET_FOLD_BUILTIN_MD_CALL?

+                 fndecl = gimple_call_fndecl (stmt);
+                 if (fndecl
+                     && TREE_CODE (fndecl) == FUNCTION_DECL

redundant check

+                     && DECL_BUILT_IN (fndecl)
+                     && !gimple_call_va_arg_pack_p (stmt)
+                     && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD)

the last check is enough, the previous two are not necessary.

+                     if (avoid_folding_inline_builtin (fndecl))
+                       break;

I don't think this is necessary.

And for simple cases such as...

+                         BUILTIN_VALL (UNOP, reduc_splus_, 10)
+                           new_tree = fold_build1 (REDUC_PLUS_EXPR,
+                                                   type,
+                                                   args[0]);
...
+             if (new_tree && ignore)
+               STRIP_NOPS (new_tree);
+
+             if (new_tree)
+               {
+                 gimplify_and_update_call_from_tree (gsi, new_tree);
+                 changed = true;
+               }

please build the resulting assingment yourself.  Like

   gimple new_stmt = gimple_build_assign_with_ops (REDUC_PLUS_EXPR,
gimple_call_lhs (call), args[0], NULL_TREE);
   gsi_replace (gsi, new_stmt, true);

instead of building a tree and then using gimplify_and_update_call_from_tree.

Thanks,
Richard.



Thanks,
Richard.

> Thanks,
>
> James Greenhalgh
> Graduate Engineer
> ARM
>
> ---
> gcc/
>
> 2013-04-19  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * coretypes.h (gimple_stmt_iterator_d): Forward declare.
>         (gimple_stmt_iterator): New typedef.
>         * doc/tm.texi.in (TARGET_FOLD_STMT): New.
>         * gimple-fold.c (fold_stmt_1): Call target hook fold_stmt.
>         * gimple.h (gimple_stmt_iterator): Rename to...
>         (gimple_stmt_iterator_d): ... This.
>         * hooks.c (hook_bool_gsiptr_bool_false): New.
>         * target.def (fold_stmt): New.
>         * doc/tm.texi: Regenerate.
>
> 2013-04-19  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * config/aarch64/aarch64-builtins.c
>         (aarch64_fold_builtin): Move folds to gimple-specific tree codes
>         to aarch64_fold_stmt.
>         (aarch64_fold_stmt): New.
>         * config/aarch64/aarch64-protos.h (aarch64_fold_stmt): New.
>         * config/aarch64/aarch64.c (TARGET_FOLD_STMT): Define.

Patch

diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index 8361632..c72863f 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -30,6 +30,9 @@ 
 #include "langhooks.h"
 #include "diagnostic-core.h"
 #include "optabs.h"
+#include "gimple.h"
+#include "tree-ssa-propagate.h"
+#include "gimple-fold.h"
 
 enum aarch64_simd_builtin_type_mode
 {
@@ -1310,21 +1313,11 @@  aarch64_fold_builtin (tree fndecl, int n_args ATTRIBUTE_UNUSED, tree *args,
 {
   int fcode = DECL_FUNCTION_CODE (fndecl);
   tree type = TREE_TYPE (TREE_TYPE (fndecl));
-
   switch (fcode)
     {
-      BUILTIN_VALL (UNOP, reduc_splus_, 10)
-	return fold_build1 (REDUC_PLUS_EXPR, type, args[0]);
-	break;
-      BUILTIN_VDQIF (UNOP, reduc_smax_, 10)
-	return fold_build1 (REDUC_MAX_EXPR, type, args[0]);
-	break;
-      BUILTIN_VDQIF (UNOP, reduc_smin_, 10)
-	return fold_build1 (REDUC_MIN_EXPR, type, args[0]);
-	break;
       BUILTIN_VDQF (UNOP, abs, 2)
 	return fold_build1 (ABS_EXPR, type, args[0]);
-	break;
+      break;
       BUILTIN_VDQF (BINOP, cmge, 0)
       BUILTIN_VDQF (BINOP, cmgt, 0)
       BUILTIN_VDQF (BINOP, cmeq, 0)
@@ -1340,15 +1333,15 @@  aarch64_fold_builtin (tree fndecl, int n_args ATTRIBUTE_UNUSED, tree *args,
 	    {
 	      BUILTIN_VDQF (BINOP, cmge, 0)
 		cmp = fold_build2 (GE_EXPR, type, args[0], args[1]);
-		break;
+	      break;
 	      BUILTIN_VDQF (BINOP, cmgt, 0)
 		cmp = fold_build2 (GT_EXPR, type, args[0], args[1]);
-		break;
+	      break;
 	      BUILTIN_VDQF (BINOP, cmeq, 0)
 		cmp = fold_build2 (EQ_EXPR, type, args[0], args[1]);
-		break;
-	      default:
-		gcc_unreachable ();
+	      break;
+	    default:
+	      gcc_unreachable ();
 	    }
 	  return fold_build3 (VEC_COND_EXPR, type, cmp, vect_true, vect_false);
 	  break;
@@ -1357,14 +1350,89 @@  aarch64_fold_builtin (tree fndecl, int n_args ATTRIBUTE_UNUSED, tree *args,
       VAR1 (UNOP, floatv4si, 2, v4sf)
       VAR1 (UNOP, floatv2di, 2, v2df)
 	return fold_build1 (FLOAT_EXPR, type, args[0]);
-	break;
-      default:
-	break;
+      break;
+    default:
+      break;
     }
 
   return NULL_TREE;
 }
 
+bool
+aarch64_fold_stmt (gimple_stmt_iterator *gsi, bool inplace)
+{
+  bool changed = false;
+  gimple stmt = gsi_stmt (*gsi);
+  tree callee;
+  tree fndecl;
+  tree new_tree = NULL_TREE;
+  if (inplace)
+    {
+      switch (gimple_code (stmt))
+	{
+	case GIMPLE_CALL:
+	    {
+	      bool ignore = (gimple_call_lhs (stmt) == NULL);
+	      callee = gimple_call_fn (stmt);
+	      if (callee)
+		{
+		  fndecl = gimple_call_fndecl (stmt);
+		  if (fndecl
+		      && TREE_CODE (fndecl) == FUNCTION_DECL
+		      && DECL_BUILT_IN (fndecl)
+		      && !gimple_call_va_arg_pack_p (stmt)
+		      && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD)
+		    {
+		      int fcode = DECL_FUNCTION_CODE (fndecl);
+		      tree type = TREE_TYPE (TREE_TYPE (fndecl));
+		      int nargs = gimple_call_num_args (stmt);
+		      tree *args = (nargs > 0
+				    ? gimple_call_arg_ptr (stmt, 0)
+				    : &error_mark_node);
+
+		      if (avoid_folding_inline_builtin (fndecl))
+			break;
+
+		      switch (fcode)
+			{
+			  BUILTIN_VALL (UNOP, reduc_splus_, 10)
+			    new_tree = fold_build1 (REDUC_PLUS_EXPR,
+						    type,
+						    args[0]);
+			  break;
+			  BUILTIN_VDQIF (UNOP, reduc_smax_, 10)
+			    new_tree = fold_build1 (REDUC_MAX_EXPR,
+						    type,
+						    args[0]);
+			  break;
+			  BUILTIN_VDQIF (UNOP, reduc_smin_, 10)
+			    new_tree = fold_build1 (REDUC_MIN_EXPR,
+						    type,
+						    args[0]);
+			  break;
+			default:
+			  break;
+			}
+		    }
+		}
+
+	      if (new_tree && ignore)
+		STRIP_NOPS (new_tree);
+
+	      if (new_tree)
+		{
+		  gimplify_and_update_call_from_tree (gsi, new_tree);
+		  changed = true;
+		}
+	      break;
+	    }
+	default:
+	  break;
+	}
+    }
+  return changed;
+}
+
 #undef AARCH64_CHECK_BUILTIN_MODE
 #undef AARCH64_FIND_FRINT_VARIANT
 #undef BUILTIN_DX
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 2400239..25503e3 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -179,6 +179,7 @@  bool aarch64_simd_mem_operand_p (rtx);
 rtx aarch64_simd_vect_par_cnst_half (enum machine_mode, bool);
 rtx aarch64_tls_get_addr (void);
 tree aarch64_fold_builtin (tree, int, tree *, bool);
+bool aarch64_fold_stmt (gimple_stmt_iterator *, bool);
 unsigned aarch64_dbx_register_number (unsigned);
 unsigned aarch64_trampoline_size (void);
 void aarch64_asm_output_labelref (FILE *, const char *);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d3b349f..3418cb0 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -7859,6 +7859,9 @@  aarch64_vectorize_vec_perm_const_ok (enum machine_mode vmode,
 #undef TARGET_FOLD_BUILTIN
 #define TARGET_FOLD_BUILTIN aarch64_fold_builtin
 
+#undef TARGET_FOLD_STMT
+#define TARGET_FOLD_STMT aarch64_fold_stmt
+
 #undef TARGET_FUNCTION_ARG
 #define TARGET_FUNCTION_ARG aarch64_function_arg