Re: [PATCH, rs6000] Fold vector absolutes in GIMPLE

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

On Wed, May 31, 2017 at 3:56 PM, Will Schmidt <will_schmidt@vnet.ibm.com> wrote:
> On Tue, 2017-05-30 at 09:00 +0200, Richard Biener wrote:
>> On Mon, May 29, 2017 at 2:21 PM, Segher Boessenkool
>> <segher@kernel.crashing.org> wrote:
>> > On Mon, May 29, 2017 at 01:35:22PM +0200, Richard Biener wrote:
>> >> >> What's the documented behavior for vec_abs with respect to an
>> >> >argument
>> >> >> of value INT_MIN?
>> >> >
>> >> >The documentation says:
>> >> >
>> >> >     "For integer vectors, the arithmetic is modular."
>> >>
>> >> This means that folding as ABS_EXPR is not safe for !TYPE_OVERFLOW_WRAPS
>> >> Integral vector types.
>> >
>> > Is it still fine if TYPE_OVERFLOW_UNDEFINED?  So essentially always
>> > except with -ftrapv?
>>
>> The docs say it needs to wrap so the correct check is TYPE_OVERFLOW_WRAPS.
>> It's not fine with TYPE_OVERFLOW_UNDEFINED as we will conclude the result
>> can never be INT_MIN while the spec says it can.
>
> Ok, thanks for the review.
>
> So it looks like I should bail with something like:
>     ...
>     case VSX_BUILTIN_XVABSDP:
>       {
>         arg0 = gimple_call_arg (stmt, 0);
>         lhs = gimple_call_lhs (stmt);
>         if (TYPE_OVERFLOW_WRAPS(TREE_TYPE(arg1))
>            return false;

No, you want

    if (! TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg1)))
      return false;

that will likely render the transform useless unless -fwrapv is given.

What we miss in the middle-end is a ABSU_EXPR that computes the
unsigned result of the absolute value (of the signed operand).  That's
always well-defined.  So you'd then lower to

y = { -2147483648, -2147483648, -2147483648, -2147483648 };
D.1234 = ABSU_EXPR <y>;
D.2579 = VIEW_CONVERT <D.1234>;

RTL expansion of ABSU_EXPR can re-use RTL abs since there's
nothing undefined on RTL.

Richard.

>         ...
>
> How can I test this scenario?  At a glance, a testcase snippet doesn't
> appear to error out.  Am I quietly losing an overflow indicator?
>
> vector signed int
> test1_min (vector signed int x)
> {
>   vector signed int y = {INT_MIN,INT_MIN,INT_MIN,INT_MIN};
>   return vec_abs (y);
> }
>
> generates gimple code:
>   y = { -2147483648, -2147483648, -2147483648, -2147483648 };
>   D.2579 = __builtin_altivec_abs_v4si (y);
> or after folding:
>   y = { -2147483648, -2147483648, -2147483648, -2147483648 };
>   D.2579 = ABS_EXPR <y>;
>
>
>
>
>>
>> Richard.
>>
>> >
>> >
>> > Segher
>>
>
>