Re: [PATCH 2/3] Simplify wrapped binops

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Jun 21, 2017 at 1:44 PM, Robin Dapp <rdapp@linux.vnet.ibm.com> wrote:
>> use INTEGRAL_TYPE_P.
>
> Done.
>
>> but you do not actually _use_ vr_outer.  Do you think that if
>> vr_outer is a VR_RANGE then the outer operation may not
>> possibly have wrapped?  That's a false conclusion.
>
> These were remains of a previous version.  vr_outer is indeed not needed
> anymore; removed.
>
>> wi::add overload with the overflow flag?  ISTR you want to handle "negative"
>> unsigned constants somehow, but then I don't see how the above works.
>> I'd say if wmin/wmax interpreted as signed are positive and then using
>> a signed op to add w1 results in a still positive number you're fine
>> (you don't seem
>> to restrict the widening cast to either zero- or sign-extending).
>
> Changed to using wi:add overload now.
>
> In essence, three cases are being handled:
>  - wrapped_range --> do not simplify
>  - !wrapped_range && ovf ("negative" unsigned) --> simplify and combine
> with sign extension in the outer type
>  - !wrapped_range && !ovf ("positive" unsigned) --> simplify and combine
> with zero extension in the outer type.

Let's split this and look at the simpler case:

+/* ((T)(A)) +- CST -> (T)(A +- CST)  */
+#if GIMPLE
+   (for outer_op (plus minus)
+    (simplify
+     (outer_op (convert SSA_NAME@0) INTEGER_CST@2)
+      (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
+          && INTEGRAL_TYPE_P (type)
+          && TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@0)))
+       /* Perform binary operation inside the cast if the constant fits
+         and there is no overflow.  */
+       (with
+       {
+         bool wrapped_range = true;
+         tree cst_inner = NULL_TREE;
+         enum value_range_type vr = VR_VARYING;
+         tree inner_type = TREE_TYPE (@0);
+
+         if (int_fits_type_p (@2, inner_type))

do

      && get_range_info (...) == VR_RANGE)

here. That avoids vr and its initialization and you get all of the "work" when
you know it will eventually succeed.

+         {
+           cst_inner = fold_convert (inner_type, @2);

ideally you'd use a wide-int here and defer the tree allocation to the result

             wide_int wi = wi::from (@2, TYPE_PRECISION (inner_type),
                                              TYPE_SIGN (inner_type));

+           wide_int wmin0, wmax0;
+           wide_int w1 = cst_inner;
+           signop sgn = TYPE_SIGN (inner_type);
+           vr = get_range_info (@0, &wmin0, &wmax0);
+
+           if (vr == VR_RANGE)
+             {
+               bool min_ovf;
+               wi::add (wmin0, w1, sgn, &min_ovf);
+
+               bool max_ovf;
+               wi::add (wmax0, w1, sgn, &max_ovf);

So I guess we never run into the outer_op == minus case as the above is
clearly wrong for that?

The comment above says "if there is no overflow" but below you allow
min_ovf && max_ovf without any further explanation.

+               wrapped_range = (min_ovf && !max_ovf) || (!min_ovf && max_ovf);
+             }
+         }
+       }
+       (if (cst_inner && !wrapped_range)
+       (convert (outer_op @0 { cst_inner; })))

thus

        (if ((min_ovf && !max_ovf) || ....)
         (convert (outer_op @0 { wide_int_to_tree (inner_type, w1); } ))))

try to keep vertical spacing in patterns minimal -- I belive that patterns
should be small enough to fit in a terminal window (24 lines).

Richard.

+       ))))
+#endif


> Regards
>  Robin