diff mbox

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

Message ID 1366391749-1765-1-git-send-email-james.greenhalgh@arm.com
State New
Headers show

Commit Message

James Greenhalgh April 19, 2013, 5:15 p.m. UTC 
Hi Richard,

Thanks for your feedback. This does feel like a much nicer solution now.

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

This seems sensible - hopefully something like the attached will be
closer to an acceptable implementation. The aarch64 portion certainly
looks much cleaner now.
 
> 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?

I've gone for TARGET_GIMPLE_FOLD_BUILTIN (to fit with the
non-target-specific gimple_fold_builtin).

I've also updated the documentation for TARGET_FOLD_BUILTIN to
make it clear that the tree returned must be valid in GENERIC
and GIMPLE.

Thanks again for your help.

James
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.
	* gimple.h (gimple_stmt_iterator): Rename to...
	(gimple_stmt_iterator_d): ... This.
	* doc/tm.texi.in (TARGET_FOLD_BUILTIN): Detail restriction that
	trees be valid for GIMPLE and GENERIC.
	(TARGET_GIMPLE_FOLD_BUILTIN): New.
	* gimple-fold.c (gimple_fold_call): Call target hook
	gimple_fold_builtin.
	* hooks.c (hook_bool_gsiptr_false): New.
	* hooks.h (hook_bool_gsiptr_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_gimple_fold_builtin.
	(aarch64_gimple_fold_builtin): New.
	* config/aarch64/aarch64-protos.h (aarch64_gimple_fold_builtin): New.
	* config/aarch64/aarch64.c (TARGET_GIMPLE_FOLD_BUILTIN): Define.

Comments

Richard Biener April 22, 2013, 11:38 a.m. UTC | #1 
On Fri, Apr 19, 2013 at 7:15 PM, James Greenhalgh
<james.greenhalgh@arm.com> wrote:
> Hi Richard,
>
> Thanks for your feedback. This does feel like a much nicer solution now.
>
>> 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.
>
> This seems sensible - hopefully something like the attached will be
> closer to an acceptable implementation. The aarch64 portion certainly
> looks much cleaner now.
>
>> 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?
>
> I've gone for TARGET_GIMPLE_FOLD_BUILTIN (to fit with the
> non-target-specific gimple_fold_builtin).
>
> I've also updated the documentation for TARGET_FOLD_BUILTIN to
> make it clear that the tree returned must be valid in GENERIC
> and GIMPLE.
>
> Thanks again for your help.

The middle-end changes are ok with moving

@@ -1145,6 +1145,13 @@ gimple_fold_call (gimple_stmt_iterator *gsi,
bool inplace)
        }
     }

+  /* Call target-specific, GIMPLE only fold routines.  */
+  callee = gimple_call_fndecl (stmt);
+  if (!changed
+      && callee
+      && DECL_BUILT_IN_CLASS (callee) == BUILT_IN_MD)
+    changed |= targetm.gimple_fold_builtin (gsi);
+
   return changed;
 }

this inside the already existing

  callee = gimple_call_fndecl (stmt);
  if (callee && DECL_BUILT_IN (callee))
    {
      tree result = gimple_fold_builtin (stmt);
      if (result)
        {
          if (!update_call_from_tree (gsi, result))
            gimplify_and_update_call_from_tree (gsi, result);
          changed = true;
        }

check - right here, as

      else if (DECL_BUILT_IN_CLASS (callee) == BUILT_IN_MD)
        changed |= targetm.gimple_fold_builtin (gsi);

also,

+/* Fold a target-specific builtin to a valid GIMPLE tree.  */
+DEFHOOK
+(gimple_fold_builtin,
+ "Like @samp{TARGET_FOLD_BUILTIN}, but without the restriction that\n\
+the result tree be valid for GENERIC.  Fold a call to a machine specific\n\
+built-in function that was set up by @samp{TARGET_INIT_BUILTINS}.\n\
+@var{gsi} points to the gimple statement holding the function call.\n\
+Returns true if any change was made to the GIMPLE stream.",

please drop the first sentence of the description.

Thanks,
Richard.


> James
> 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.
>         * gimple.h (gimple_stmt_iterator): Rename to...
>         (gimple_stmt_iterator_d): ... This.
>         * doc/tm.texi.in (TARGET_FOLD_BUILTIN): Detail restriction that
>         trees be valid for GIMPLE and GENERIC.
>         (TARGET_GIMPLE_FOLD_BUILTIN): New.
>         * gimple-fold.c (gimple_fold_call): Call target hook
>         gimple_fold_builtin.
>         * hooks.c (hook_bool_gsiptr_false): New.
>         * hooks.h (hook_bool_gsiptr_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_gimple_fold_builtin.
>         (aarch64_gimple_fold_builtin): New.
>         * config/aarch64/aarch64-protos.h (aarch64_gimple_fold_builtin): New.
>         * config/aarch64/aarch64.c (TARGET_GIMPLE_FOLD_BUILTIN): Define.
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index 8361632..7a65d59 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,70 @@  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_gimple_fold_builtin (gimple_stmt_iterator *gsi)
+{
+  bool changed = false;
+  gimple stmt = gsi_stmt (*gsi);
+  tree call = gimple_call_fn (stmt);
+  tree fndecl;
+  gimple new_stmt = NULL;
+  if (call)
+    {
+      fndecl = gimple_call_fndecl (stmt);
+      if (fndecl)
+	{
+	  int fcode = DECL_FUNCTION_CODE (fndecl);
+	  int nargs = gimple_call_num_args (stmt);
+	  tree *args = (nargs > 0
+			? gimple_call_arg_ptr (stmt, 0)
+			: &error_mark_node);
+
+	  switch (fcode)
+	    {
+	      BUILTIN_VALL (UNOP, reduc_splus_, 10)
+		new_stmt = gimple_build_assign_with_ops (
+						REDUC_PLUS_EXPR,
+						gimple_call_lhs (stmt),
+						args[0],
+						NULL_TREE);
+		break;
+	      BUILTIN_VDQIF (UNOP, reduc_smax_, 10)
+		new_stmt = gimple_build_assign_with_ops (
+						REDUC_MAX_EXPR,
+						gimple_call_lhs (stmt),
+						args[0],
+						NULL_TREE);
+		break;
+	      BUILTIN_VDQIF (UNOP, reduc_smin_, 10)
+		new_stmt = gimple_build_assign_with_ops (
+						REDUC_MIN_EXPR,
+						gimple_call_lhs (stmt),
+						args[0],
+						NULL_TREE);
+		break;
+	    default:
+	      break;
+	    }
+	}
+    }
+
+  if (new_stmt)
+    {
+      gsi_replace (gsi, new_stmt, true);
+      changed = true;
+    }
+  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..4a53f32 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_gimple_fold_builtin (gimple_stmt_iterator *);
 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..b9535bf 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_GIMPLE_FOLD_BUILTIN
+#define TARGET_GIMPLE_FOLD_BUILTIN aarch64_gimple_fold_builtin
+
 #undef TARGET_FUNCTION_ARG
 #define TARGET_FUNCTION_ARG aarch64_function_arg