On 11/04/13 08:44, Jakub Jelinek wrote:
I guess short time yes, but I wonder if it wouldn't be better to use
walk_gimple_op and do all changes in the callback. Instead of passing
adjustments pass around a struct containing the adjustments, current stmt
and the modified flag. You can use the val_only and is_lhs to determine
what you need to do (probably need to reset those two for the subtrees
to val_only = true, is_lhs = false and not walk subtrees of types) and you
could (if not val_only) immediately gimplify it properly (insert temporary
SSA_NAME setter before resp. store after depending on is_lhs).
Then you could avoid the regimplification.
With the attached patch, we get rid of the ad-hoc argument adjustment
system that is in place, and avoid regimplification altogether. I am
using walk_gimple_op as suggested, thus cleaning up most of
ipa_simd_modify_function_body.
I have checked the following patch with the attached testcases that were
previously ICEing, and with a handful of handcrafted tests that I
checked manually (array references on lhs and rhs, vectors of pointers,
etc).
OK for branch?
gcc/ChangeLog.gomp
* ipa-prop.h (sra_ipa_get_adjustment_candidate): Protoize.
* omp-low.c (struct modify_stmt_info): New.
(ipa_simd_modify_function_body_ops_1): Remove.
(ipa_simd_modify_stmt_ops): New.
(ipa_simd_modify_function_body_ops): Remove.
(ipa_simd_modify_function_body): Set new callback info.
Remove special casing. Handle all operators with walk_gimple_op.
* tree-sra.c (get_ssa_base_param): Add new argument. Use it.
(disqualify_base_of_expr): Pass new argument to
get_ssa_base_param.
(sra_ipa_modify_expr): Abstract candidate search into...
(sra_ipa_get_adjustment_candidate): ...here.
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 06296d1..6aebf8d 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -719,5 +719,8 @@ tree build_ref_for_offset (location_t, tree, HOST_WIDE_INT,
tree,
gimple_stmt_iterator *, bool);
bool ipa_sra_modify_function_body (ipa_parm_adjustment_vec);
bool sra_ipa_modify_expr (tree *, bool, ipa_parm_adjustment_vec);
+ipa_parm_adjustment *sra_ipa_get_adjustment_candidate (tree *&, bool *,
+ ipa_parm_adjustment_vec,
+ bool);
#endif /* IPA_PROP_H */
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 94058af..0dd0676 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -11049,33 +11049,81 @@ simd_clone_init_simd_arrays (struct cgraph_node *node,
return seq;
}
+/* Callback info for ipa_simd_modify_stmt_ops below. */
+
+struct modify_stmt_info {
+ ipa_parm_adjustment_vec adjustments;
+ gimple stmt;
+ /* True if the parent statement was modified by
+ ipa_simd_modify_stmt_ops. */
+ bool modified;
+};
+
+/* Callback for walk_gimple_op.
+
+ Adjust operands from a given statement as specified in the
+ adjustments vector in the callback data. */
+
static tree
-ipa_simd_modify_function_body_ops_1 (tree *tp, int *walk_subtrees, void *data)
+ipa_simd_modify_stmt_ops (tree *tp,
+ int *walk_subtrees ATTRIBUTE_UNUSED,
+ void *data)
{
struct walk_stmt_info *wi = (struct walk_stmt_info *) data;
- ipa_parm_adjustment_vec *adjustments = (ipa_parm_adjustment_vec *) wi->info;
+ if (!SSA_VAR_P (*tp))
+ {
+ /* Make sure we treat subtrees as a RHS. This makes sure that
+ when examining the `*foo' in *foo=x, the `foo' get treated as
+ a use properly. */
+ wi->is_lhs = false;
+ wi->val_only = true;
+ return NULL_TREE;
+ }
+ struct modify_stmt_info *info = (struct modify_stmt_info *) wi->info;
+ struct ipa_parm_adjustment *cand
+ = sra_ipa_get_adjustment_candidate (tp, NULL, info->adjustments, true);
+ if (!cand)
+ return NULL_TREE;
+
tree t = *tp;
+ tree repl = make_ssa_name (create_tmp_var (TREE_TYPE (t), NULL), NULL);
- if (DECL_P (t) || TREE_CODE (t) == SSA_NAME)
- return (tree) sra_ipa_modify_expr (tp, true, *adjustments);
+ gimple stmt;
+ gimple_stmt_iterator gsi = gsi_for_stmt (info->stmt);
+ if (wi->is_lhs)
+ {
+ stmt = gimple_build_assign (unshare_expr (cand->reduction), repl);
+ gsi_insert_after (&gsi, stmt, GSI_SAME_STMT);
+ SSA_NAME_DEF_STMT (repl) = info->stmt;
+ }
else
- *walk_subtrees = 1;
- return NULL_TREE;
-}
+ {
+ /* You'd think we could skip the extra SSA variable when
+ wi->val_only=true, but we may have `*var' which will get
+ replaced into `*var_array[iter]' and will likely be something
+ not gimple. */
+ stmt = gimple_build_assign (repl, unshare_expr (cand->reduction));
+ gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
+ }
-/* Helper function for ipa_simd_modify_function_body. Make any
- necessary adjustments for tree operators. */
+ if (!useless_type_conversion_p (TREE_TYPE (*tp), TREE_TYPE (repl)))
+ {
+ tree vce = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (*tp), repl);
+ *tp = vce;
+ }
+ else
+ *tp = repl;
-static bool
-ipa_simd_modify_function_body_ops (tree *tp,
- ipa_parm_adjustment_vec *adjustments)
-{
- struct walk_stmt_info wi;
- memset (&wi, 0, sizeof (wi));
- wi.info = adjustments;
- bool res = (bool) walk_tree (tp, ipa_simd_modify_function_body_ops_1,
- &wi, NULL);
- return res;
+ /* We have modified in place; update the SSA operands. */
+ info->modified = false;
+ stmt = info->stmt;
+ update_stmt (stmt);
+ if (maybe_clean_eh_stmt (stmt))
+ gimple_purge_dead_eh_edges (gimple_bb (stmt));
+
+ wi->is_lhs = false;
+ wi->val_only = true;
+ return NULL_TREE;
}
/* Traverse the function body and perform all modifications as
@@ -11111,6 +11159,9 @@ ipa_simd_modify_function_body (struct cgraph_node *node,
NULL_TREE, NULL_TREE);
}
+ struct modify_stmt_info info;
+ info.adjustments = adjustments;
+
FOR_EACH_BB_FN (bb, DECL_STRUCT_FUNCTION (node->decl))
{
gimple_stmt_iterator gsi;
@@ -11119,9 +11170,13 @@ ipa_simd_modify_function_body (struct cgraph_node
*node,
while (!gsi_end_p (gsi))
{
gimple stmt = gsi_stmt (gsi);
+ info.stmt = stmt;
bool modified = false;
- tree *t;
- unsigned i;
+ struct walk_stmt_info wi;
+
+ memset (&wi, 0, sizeof (wi));
+ info.modified = false;
+ wi.info = &info;
switch (gimple_code (stmt))
{
@@ -11147,56 +11202,9 @@ ipa_simd_modify_function_body (struct cgraph_node
*node,
break;
}
- case GIMPLE_ASSIGN:
- t = gimple_assign_lhs_ptr (stmt);
- modified |= sra_ipa_modify_expr (t, false, adjustments);
-
- /* The LHS may have operands that also need adjusting
- (e.g. `foo' in array[foo]). */
- modified |= ipa_simd_modify_function_body_ops (t, &adjustments);
-
- for (i = 0; i < gimple_num_ops (stmt); ++i)
- {
- t = gimple_op_ptr (stmt, i);
- modified |= sra_ipa_modify_expr (t, false, adjustments);
- }
- break;
-
- case GIMPLE_CALL:
- /* Operands must be processed before the lhs. */
- for (i = 0; i < gimple_call_num_args (stmt); i++)
- {
- t = gimple_call_arg_ptr (stmt, i);
- modified |= sra_ipa_modify_expr (t, true, adjustments);
- }
-
- if (gimple_call_lhs (stmt))
- {
- t = gimple_call_lhs_ptr (stmt);
- modified |= sra_ipa_modify_expr (t, false, adjustments);
- }
- break;
-
- case GIMPLE_ASM:
- for (i = 0; i < gimple_asm_ninputs (stmt); i++)
- {
- t = &TREE_VALUE (gimple_asm_input_op (stmt, i));
- modified |= sra_ipa_modify_expr (t, true, adjustments);
- }
- for (i = 0; i < gimple_asm_noutputs (stmt); i++)
- {
- t = &TREE_VALUE (gimple_asm_output_op (stmt, i));
- modified |= sra_ipa_modify_expr (t, false, adjustments);
- }
- break;
-
default:
- for (i = 0; i < gimple_num_ops (stmt); ++i)
- {
- t = gimple_op_ptr (stmt, i);
- if (*t)
- modified |= sra_ipa_modify_expr (t, true, adjustments);
- }
+ walk_gimple_op (stmt, ipa_simd_modify_stmt_ops, &wi);
+ modified |= info.modified;
break;
}
diff --git a/gcc/testsuite/gcc.dg/gomp/simd-clones-7.c
b/gcc/testsuite/gcc.dg/gomp/simd-clones-7.c
new file mode 100644
index 0000000..ef6fa11
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/gomp/simd-clones-7.c
@@ -0,0 +1,16 @@
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-fopenmp -w" } */
+
+int array[1000];
+
+#pragma omp declare simd notinbranch simdlen(4)
+void foo (int *a, int b)
+{
+ a[b] = 555;
+}
+
+#pragma omp declare simd notinbranch simdlen(4)
+void bar (int *a)
+{
+ *a = 555;
+}
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 27938ab..36994f7 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -785,15 +785,17 @@ type_internals_preclude_sra_p (tree type, const char
**msg)
}
}
-/* If T is an SSA_NAME, return NULL if it is not a default def or return its
- base variable if it is. Return T if it is not an SSA_NAME. */
+/* If T is an SSA_NAME, return NULL if it is not a default def or
+ return its base variable if it is. If IGNORE_DEFAULT_DEF is true,
+ the base variable is always returned, regardless if it is a default
+ def. Return T if it is not an SSA_NAME. */
static tree
-get_ssa_base_param (tree t)
+get_ssa_base_param (tree t, bool ignore_default_def)
{
if (TREE_CODE (t) == SSA_NAME)
{
- if (SSA_NAME_IS_DEFAULT_DEF (t))
+ if (ignore_default_def || SSA_NAME_IS_DEFAULT_DEF (t))
return SSA_NAME_VAR (t);
else
return NULL_TREE;
@@ -874,7 +876,7 @@ create_access (tree expr, gimple stmt, bool write)
if (sra_mode == SRA_MODE_EARLY_IPA
&& TREE_CODE (base) == MEM_REF)
{
- base = get_ssa_base_param (TREE_OPERAND (base, 0));
+ base = get_ssa_base_param (TREE_OPERAND (base, 0), false);
if (!base)
return NULL;
ptr = true;
@@ -1039,7 +1041,7 @@ disqualify_base_of_expr (tree t, const char *reason)
t = get_base_address (t);
if (sra_mode == SRA_MODE_EARLY_IPA
&& TREE_CODE (t) == MEM_REF)
- t = get_ssa_base_param (TREE_OPERAND (t, 0));
+ t = get_ssa_base_param (TREE_OPERAND (t, 0), false);
if (t && DECL_P (t))
disqualify_candidate (t, reason);
@@ -4485,35 +4487,35 @@ replace_removed_params_ssa_names (gimple stmt,
return true;
}
-/* If the expression *EXPR should be replaced by a reduction of a parameter, do
- so. ADJUSTMENTS is a pointer to a vector of adjustments. CONVERT
- specifies whether the function should care about type incompatibility the
- current and new expressions. If it is false, the function will leave
- incompatibility issues to the caller. Return true iff the expression
- was modified. */
+/* Given an expression, return an adjustment entry specifying the
+ transformation to be done on EXPR. If no suitable adjustment entry
+ was found, returns NULL.
-bool
-sra_ipa_modify_expr (tree *expr, bool convert,
- ipa_parm_adjustment_vec adjustments)
-{
- int i, len;
- struct ipa_parm_adjustment *adj, *cand = NULL;
- HOST_WIDE_INT offset, size, max_size;
- tree base, src;
+ If IGNORE_DEFAULT_DEF is set, consider SSA_NAMEs which are not a
+ default def, otherwise bail on them.
- len = adjustments.length ();
+ If CONVERT is non-NULL, this function will set *CONVERT if the
+ expression provided is a component reference that must be converted
+ upon return. ADJUSTMENTS is the adjustments vector. */
+ipa_parm_adjustment *
+sra_ipa_get_adjustment_candidate (tree *&expr, bool *convert,
+ ipa_parm_adjustment_vec adjustments,
+ bool ignore_default_def)
+{
if (TREE_CODE (*expr) == BIT_FIELD_REF
|| TREE_CODE (*expr) == IMAGPART_EXPR
|| TREE_CODE (*expr) == REALPART_EXPR)
{
expr = &TREE_OPERAND (*expr, 0);
- convert = true;
+ if (convert)
+ *convert = true;
}
- base = get_ref_base_and_extent (*expr, &offset, &size, &max_size);
+ HOST_WIDE_INT offset, size, max_size;
+ tree base = get_ref_base_and_extent (*expr, &offset, &size, &max_size);
if (!base || size == -1 || max_size == -1)
- return false;
+ return NULL;
if (TREE_CODE (base) == MEM_REF)
{
@@ -4521,13 +4523,15 @@ sra_ipa_modify_expr (tree *expr, bool convert,
base = TREE_OPERAND (base, 0);
}
- base = get_ssa_base_param (base);
+ base = get_ssa_base_param (base, ignore_default_def);
if (!base || TREE_CODE (base) != PARM_DECL)
- return false;
+ return NULL;
- for (i = 0; i < len; i++)
+ struct ipa_parm_adjustment *cand = NULL;
+ unsigned int len = adjustments.length ();
+ for (unsigned i = 0; i < len; i++)
{
- adj = &adjustments[i];
+ struct ipa_parm_adjustment *adj = &adjustments[i];
if (adj->base == base
&& (adj->offset == offset || adj->remove_param))
@@ -4536,9 +4540,29 @@ sra_ipa_modify_expr (tree *expr, bool convert,
break;
}
}
+
if (!cand || cand->copy_param || cand->remove_param)
+ return NULL;
+ return cand;
+}
+
+/* If the expression *EXPR should be replaced by a reduction of a parameter, do
+ so. ADJUSTMENTS is a pointer to a vector of adjustments. CONVERT
+ specifies whether the function should care about type incompatibility the
+ current and new expressions. If it is false, the function will leave
+ incompatibility issues to the caller. Return true iff the expression
+ was modified. */
+
+bool
+sra_ipa_modify_expr (tree *expr, bool convert,
+ ipa_parm_adjustment_vec adjustments)
+{
+ struct ipa_parm_adjustment *cand
+ = sra_ipa_get_adjustment_candidate (expr, &convert, adjustments, false);
+ if (!cand)
return false;
+ tree src;
if (cand->by_ref)
src = build_simple_mem_ref (cand->reduction);
else
