Re: [PATCH PR80153]Always generate folded type conversion in tree-affine

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Mar 28, 2017 at 2:01 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
> Hi,
> This patch is to fix PR80153.  As analyzed in the PR, root cause is tree_affine lacks
> ability differentiating (unsigned)(ptr + offset) and (unsigned)ptr + (unsigned)offset,
> even worse, it always returns the former expression in aff_combination_tree, which
> is wrong if the original expression has the latter form.  The patch resolves the issue
> by always returning the latter form expression, i.e, always trying to generate folded
> expression.  Also as analyzed in comment, I think this change won't result in substantial
> code gen difference.
> I also need to adjust get_computation_aff for test case gcc.dg/tree-ssa/reassoc-19.c.
> Well, I think the changed behavior is correct, but for case the original pointer candidate
> is chosen, it should be unnecessary to compute in uutype.  Also this adjustment only
> generates (unsigned)(pointer + offset) which is generated by tree-affine.c.
> Bootstrap and test on x86_64 and AArch64.  Is it OK?

Hmm.  What is the desired goal?  To have all elts added have
comb->type as type?  Then
the type passed to add_elt_to_tree is redundant with comb->type.  It
looks like it
is always passed comb->type now.

ISTR from past work in this area that it was important for pointer
combinations to allow
both pointer and sizetype elts at least.

Your change is incomplete I think, for the scale == -1 and POINTER_TYPE_P case
elt is sizetype now, not of pointer type.  As said above, we are
trying to maintain
both pointer and sizetype elts with like:

  if (scale == 1)
    {
      if (!expr)
        {
          if (POINTER_TYPE_P (TREE_TYPE (elt)))
            return elt;
          else
            return fold_convert (type1, elt);
        }

where your earilier fold to type would result in not all cases handled the same
(depending whether scale was -1 for example).

Thus - shouldn't we simply drop the type argument (or rather the comb one?
that wide_int_ext_for_comb looks weird given we get a widest_int as input
and all the other wide_int_ext_for_comb calls around).

And unconditionally convert to type, simplifying the rest of the code?

Richard.


> 2017-03-27  Bin Cheng  <bin.cheng@arm.com>
>
>         PR tree-optimization/80153
>         * tree-affine.c (add_elt_to_tree): Convert to type as required
>         by function's parameter.
>         * tree-ssa-loop-ivopts.c (alloc_iv): Pass in consistent types.
>         (get_computation_aff): Use utype directly for original candidate.
>
> gcc/testsuite/ChangeLog
> 2017-03-27  Bin Cheng  <bin.cheng@arm.com>
>
>         PR tree-optimization/80153
>         * gcc.c-torture/execute/pr80153.c: New.