On Wed, Nov 06, 2013 at 03:24:40PM -0700, Aldy Hernandez wrote:
> 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).
I'd like Martin to eyeball the ipa changes eventually.
> OK for branch?
Ok with a few nits fixed:
> 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. */
I think it wouldn't hurt to e.g. do
if (TYPE_P (*tp))
*walk_subtrees = 0;
here (and drop ATTRIBUTE_UNUSED above.
> + 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);
Just do
tree repl = make_ssa_name (TREE_TYPE (t), NULL);
no need to create underlying vars since 4.8.
> + /* We have modified in place; update the SSA operands. */
> + info->modified = false;
So you always set modified to false? I was expecting you'd set
it to true here and defer update_stmt and maybe_clean_eh_stmt etc.
to after walk_gimple_op (so, do it only when all the changes on the stmt
have been performed). Plus, when you modify something, there is no need
to walk subtrees, so you can just do *walk_subtrees = 0; too.
Jakub
