This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [gomp4] rewrite simd clone argument adjustment to avoid regimplification


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]