wide-int, tree-ssa

Only a partial reply.  I'll leave Kenny and Mike to answer the VARYING
question.

Richard Biener <richard.guenther@gmail.com> writes:
> On Sat, Nov 23, 2013 at 8:23 PM, Mike Stump <mikestump@comcast.net> wrote:
>> Richi has asked the we break the wide-int patch so that the individual
>> port and front end maintainers can review their parts without have to
>> go through the entire patch.  This patch covers the tree-saa code.
>>
>> Ok?
>
> Same comments as to tree-affine.c apply to tree-ssa-address.c.
>
> @@ -887,8 +886,8 @@ copy_ref_info (tree new_ref, tree old_ref)
>                            && (TREE_INT_CST_LOW (TMR_STEP (new_ref))
>                                < align)))))
>             {
> -             unsigned int inc = (mem_ref_offset (old_ref)
> -                                 - mem_ref_offset (new_ref)).low;
> +             unsigned int inc = (mem_ref_offset (old_ref).to_uhwi ()
> +                                 - mem_ref_offset (new_ref).to_uhwi ());
>               adjust_ptr_info_misalignment (new_pi, inc);
>             }
>           else
>
> other patches used .to_short_addr (), also your conversion isn't
> correct - previously the subtraction was in infinite precision and only
> the result (possibly) truncated.  Now the subtraction operands are
> truncated - that looks wrong.

Sorry, forgot about .to_short_addr () when doing the merge.
The conversion should be OK with that fixed though, since truncation
distributes through subtraction (and truncating first is cheaper).

> +/* Return a widest_int that can be used for bitwise simplifications
>     from VAL.  */
>
> -static double_int
> -value_to_double_int (prop_value_t val)
> +static widest_int
> +value_to_wide_int (prop_value_t val)
>  {
>    if (val.value
>        && TREE_CODE (val.value) == INTEGER_CST)
> -    return tree_to_double_int (val.value);
> -  else
> -    return double_int_zero;
> +    return wi::to_widest (val.value);
> +
> +  return 0;
>  }
>
> you also get to hope that we optimize all the widest_int return-by-value
> code to elide the implied copying ... (not sure if you can do that by
> adding a not implemented copy / assignment constructor - C++ folks?)

Are you worried about a copy from one widest_int to another?
Won't the return-value optimisation stop that?

Or are you worried about the copy from the tree to the widest_int?
In this particular case I suppose we could use:

  wi::to_widest (integer_zero_node)

instead of 0 and return a:

  generic_wide_int <extended_tree <MAX_BITSIZE_MODE_ANY_INT> >

(typedefed :-)).  Is the function really hot enough to justify the
uglification though?  We're thinking about these kinds of tweaks now
because it's flavour of the month, but I bet post-merge patches will
use the more obvious implementation instead.

OTOH maybe this is a natural candidate for C++11 auto...

>      case LT_EXPR:
>      case LE_EXPR:
>        {
> +       widest_int o1val, o2val, o1mask, o2mask;
>         int minmax, maxmin;
> +
> +       if ((code == GE_EXPR) || (code == GT_EXPR))
> +         {
> +           o1val = r2val;
> +           o1mask = r2mask;
> +           o2val = r1val;
> +           o2mask = r1mask;
> +           code = swap_tree_comparison (code);
> +         }
> +       else
> +         {
> +           o1val = r1val;
> +           o1mask = r1mask;
> +           o2val = r2val;
> +           o2mask = r2mask;
> +         }
>
> that are pretty expensive copies, no?  Consider using
>
>   const widest_int &o1val = swap ? r2val : r1val;
>
> and so on.  (C++ folks?  Any idea how to avoid the repeated
> conditional init in favor of sth that more resembles the original?)

Not a C++ person, but I can't think of one either.  It probably
wouldn't be as readable as the four separate statements though.

> diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
> index 6ea634c..c975a97 100644
> --- a/gcc/tree-ssa-loop-im.c
> +++ b/gcc/tree-ssa-loop-im.c
> @@ -1643,7 +1643,7 @@ mem_refs_may_alias_p (mem_ref_p mem1, mem_ref_p mem2,
>    /* Perform BASE + OFFSET analysis -- if MEM1 and MEM2 are based on the same
>       object and their offset differ in such a way that the locations cannot
>       overlap, then they cannot alias.  */
> -  double_int size1, size2;
> +  widest_int size1, size2;
>    aff_tree off1, off2;
>
> from the names you can know this is offset_int.

I agree that's true in the overlap and get_inner_reference_aff cases,
but most of tree-affine seems to be more general than that.  Is it really
worth bunging in conversions between offset_int and widest_int to save a
few HWIs of stack space?

> @@ -529,15 +526,15 @@ end:
>     difference of two values in TYPE.  */
>
>  static void
> -bounds_add (bounds *bnds, double_int delta, tree type)
> +bounds_add (bounds *bnds, widest_int delta, tree type)
>  {
>    mpz_t mdelta, max;
>
> const widest_int &delta please (please double-check the patches for
> widest-int-by-value passing).
>
> @@ -2592,7 +2587,7 @@ derive_constant_upper_bound_ops (tree type, tree op0,
>
>  static void
>  do_warn_aggressive_loop_optimizations (struct loop *loop,
> -                                      double_int i_bound, gimple stmt)
> +                                      widest_int i_bound, gimple stmt)
>  {
>    /* Don't warn if the loop doesn't have known constant bound.  */
>    if (!loop->nb_iterations
>
> Likewise.

Fixed, along with a few other cases.

Testing still in progress, but does this look OK?

Thanks,
Richard

wide-int, tree-ssa

Commit Message

Comments

Patch