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

[Ramana Radhakrishnan <ramana.gcc@googlemail.com>]

On Wed, May 31, 2017 at 3:02 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> 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.


There is a PR for this in BZ, though can't find it in a quick search
... We can use this on arm and aarch64 as well IIRC.

regards
Ramana

>
> 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
>>>
>>
>>