* [PATCH, rs6000] Fold vector absolutes in GIMPLE
@ 2017-05-26 17:19 Will Schmidt
2017-05-29 8:39 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Will Schmidt @ 2017-05-26 17:19 UTC (permalink / raw)
To: GCC Patches, Segher Boessenkool, David Edelsohn; +Cc: Bill Schmidt
Hi,
Add support for early expansion of vector absolute built-ins.
Bootstraps currently running (p7,p8le,p8be).
OK for trunk?
Thanks,
-Will
[gcc]
2017-05-26 Will Schmidt <will_schmidt@vnet.ibm.com>
* config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add handling
for early expansion of vector absolute builtins.
[gcc/testsuite]
2017-05-15 Will Schmidt <will_schmidt@vnet.ibm.com>
* gcc.target/powerpc/fold-vec-abs-char.c: New.
* gcc.target/powerpc/fold-vec-abs-floatdouble.c: New.
* gcc.target/powerpc/fold-vec-abs-int.c: New.
* gcc.target/powerpc/fold-vec-abs-longlong.c: New.
* gcc.target/powerpc/fold-vec-abs-short.c: New.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index dac673c..104a052 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -17333,6 +17333,21 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
gsi_replace (gsi, g, true);
return true;
}
+ /* flavors of vec_abs. */
+ case ALTIVEC_BUILTIN_ABS_V16QI:
+ case ALTIVEC_BUILTIN_ABS_V8HI:
+ case ALTIVEC_BUILTIN_ABS_V4SI:
+ case ALTIVEC_BUILTIN_ABS_V4SF:
+ case P8V_BUILTIN_ABS_V2DI:
+ case VSX_BUILTIN_XVABSDP:
+ {
+ arg0 = gimple_call_arg (stmt, 0);
+ lhs = gimple_call_lhs (stmt);
+ gimple *g = gimple_build_assign (lhs, ABS_EXPR, arg0);
+ gimple_set_location (g, gimple_location (stmt));
+ gsi_replace (gsi, g, true);
+ return true;
+ }
default:
break;
}
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-char.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-char.c
new file mode 100644
index 0000000..239c919
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-char.c
@@ -0,0 +1,18 @@
+/* Verify that overloaded built-ins for vec_abs with char
+ inputs produce the right results. */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec -O2" } */
+
+#include <altivec.h>
+
+vector signed char
+test2 (vector signed char x)
+{
+ return vec_abs (x);
+}
+
+/* { dg-final { scan-assembler-times "vspltisw|vxor" 1 } } */
+/* { dg-final { scan-assembler-times "vsububm" 1 } } */
+/* { dg-final { scan-assembler-times "vmaxsb" 1 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-floatdouble.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-floatdouble.c
new file mode 100644
index 0000000..1a08618
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-floatdouble.c
@@ -0,0 +1,23 @@
+/* Verify that overloaded built-ins for vec_abs with float and
+ double inputs for VSX produce the right results. */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-mvsx -O2" } */
+
+#include <altivec.h>
+
+vector float
+test1 (vector float x)
+{
+ return vec_abs (x);
+}
+
+vector double
+test2 (vector double x)
+{
+ return vec_abs (x);
+}
+
+/* { dg-final { scan-assembler-times "xvabssp" 1 } } */
+/* { dg-final { scan-assembler-times "xvabsdp" 1 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-int.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-int.c
new file mode 100644
index 0000000..caf8861
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-int.c
@@ -0,0 +1,18 @@
+/* Verify that overloaded built-ins for vec_abs with int
+ inputs produce the right results. */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec -O2 " } */
+
+#include <altivec.h>
+
+vector signed int
+test1 (vector signed int x)
+{
+ return vec_abs (x);
+}
+
+/* { dg-final { scan-assembler-times "vspltisw|vxor" 1 } } */
+/* { dg-final { scan-assembler-times "vsubuwm" 1 } } */
+/* { dg-final { scan-assembler-times "vmaxsw" 1 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-longlong.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-longlong.c
new file mode 100644
index 0000000..5b59d19
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-longlong.c
@@ -0,0 +1,18 @@
+/* Verify that overloaded built-ins for vec_abs with long long
+ inputs produce the right results. */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-options "-mpower8-vector -O2" } */
+
+#include <altivec.h>
+
+vector signed long long
+test3 (vector signed long long x)
+{
+ return vec_abs (x);
+}
+
+/* { dg-final { scan-assembler-times "vspltisw|vxor" 1 } } */
+/* { dg-final { scan-assembler-times "vsubudm" 1 } } */
+/* { dg-final { scan-assembler-times "vmaxsd" 1 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-short.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-short.c
new file mode 100644
index 0000000..d312000
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-short.c
@@ -0,0 +1,18 @@
+/* Verify that overloaded built-ins for vec_abs with short
+ inputs produce the right results. */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec -O2" } */
+
+#include <altivec.h>
+
+vector signed short
+test3 (vector signed short x)
+{
+ return vec_abs (x);
+}
+
+/* { dg-final { scan-assembler-times "vspltisw|vxor" 1 } } */
+/* { dg-final { scan-assembler-times "vsubuhm" 1 } } */
+/* { dg-final { scan-assembler-times "vmaxsh" 1 } } */
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH, rs6000] Fold vector absolutes in GIMPLE
2017-05-26 17:19 [PATCH, rs6000] Fold vector absolutes in GIMPLE Will Schmidt
@ 2017-05-29 8:39 ` Richard Biener
2017-05-29 10:29 ` Segher Boessenkool
0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2017-05-29 8:39 UTC (permalink / raw)
To: will_schmidt
Cc: GCC Patches, Segher Boessenkool, David Edelsohn, Bill Schmidt
On Fri, May 26, 2017 at 7:19 PM, Will Schmidt <will_schmidt@vnet.ibm.com> wrote:
> Hi,
>
> Add support for early expansion of vector absolute built-ins.
>
> Bootstraps currently running (p7,p8le,p8be).
>
> OK for trunk?
What's the documented behavior for vec_abs with respect to an argument
of value INT_MIN?
Richard.
> Thanks,
> -Will
>
>
> [gcc]
>
> 2017-05-26 Will Schmidt <will_schmidt@vnet.ibm.com>
>
> * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add handling
> for early expansion of vector absolute builtins.
>
> [gcc/testsuite]
>
> 2017-05-15 Will Schmidt <will_schmidt@vnet.ibm.com>
>
> * gcc.target/powerpc/fold-vec-abs-char.c: New.
> * gcc.target/powerpc/fold-vec-abs-floatdouble.c: New.
> * gcc.target/powerpc/fold-vec-abs-int.c: New.
> * gcc.target/powerpc/fold-vec-abs-longlong.c: New.
> * gcc.target/powerpc/fold-vec-abs-short.c: New.
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index dac673c..104a052 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -17333,6 +17333,21 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
> gsi_replace (gsi, g, true);
> return true;
> }
> + /* flavors of vec_abs. */
> + case ALTIVEC_BUILTIN_ABS_V16QI:
> + case ALTIVEC_BUILTIN_ABS_V8HI:
> + case ALTIVEC_BUILTIN_ABS_V4SI:
> + case ALTIVEC_BUILTIN_ABS_V4SF:
> + case P8V_BUILTIN_ABS_V2DI:
> + case VSX_BUILTIN_XVABSDP:
> + {
> + arg0 = gimple_call_arg (stmt, 0);
> + lhs = gimple_call_lhs (stmt);
> + gimple *g = gimple_build_assign (lhs, ABS_EXPR, arg0);
> + gimple_set_location (g, gimple_location (stmt));
> + gsi_replace (gsi, g, true);
> + return true;
> + }
> default:
> break;
> }
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-char.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-char.c
> new file mode 100644
> index 0000000..239c919
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-char.c
> @@ -0,0 +1,18 @@
> +/* Verify that overloaded built-ins for vec_abs with char
> + inputs produce the right results. */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_altivec_ok } */
> +/* { dg-options "-maltivec -O2" } */
> +
> +#include <altivec.h>
> +
> +vector signed char
> +test2 (vector signed char x)
> +{
> + return vec_abs (x);
> +}
> +
> +/* { dg-final { scan-assembler-times "vspltisw|vxor" 1 } } */
> +/* { dg-final { scan-assembler-times "vsububm" 1 } } */
> +/* { dg-final { scan-assembler-times "vmaxsb" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-floatdouble.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-floatdouble.c
> new file mode 100644
> index 0000000..1a08618
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-floatdouble.c
> @@ -0,0 +1,23 @@
> +/* Verify that overloaded built-ins for vec_abs with float and
> + double inputs for VSX produce the right results. */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-mvsx -O2" } */
> +
> +#include <altivec.h>
> +
> +vector float
> +test1 (vector float x)
> +{
> + return vec_abs (x);
> +}
> +
> +vector double
> +test2 (vector double x)
> +{
> + return vec_abs (x);
> +}
> +
> +/* { dg-final { scan-assembler-times "xvabssp" 1 } } */
> +/* { dg-final { scan-assembler-times "xvabsdp" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-int.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-int.c
> new file mode 100644
> index 0000000..caf8861
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-int.c
> @@ -0,0 +1,18 @@
> +/* Verify that overloaded built-ins for vec_abs with int
> + inputs produce the right results. */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_altivec_ok } */
> +/* { dg-options "-maltivec -O2 " } */
> +
> +#include <altivec.h>
> +
> +vector signed int
> +test1 (vector signed int x)
> +{
> + return vec_abs (x);
> +}
> +
> +/* { dg-final { scan-assembler-times "vspltisw|vxor" 1 } } */
> +/* { dg-final { scan-assembler-times "vsubuwm" 1 } } */
> +/* { dg-final { scan-assembler-times "vmaxsw" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-longlong.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-longlong.c
> new file mode 100644
> index 0000000..5b59d19
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-longlong.c
> @@ -0,0 +1,18 @@
> +/* Verify that overloaded built-ins for vec_abs with long long
> + inputs produce the right results. */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> +/* { dg-options "-mpower8-vector -O2" } */
> +
> +#include <altivec.h>
> +
> +vector signed long long
> +test3 (vector signed long long x)
> +{
> + return vec_abs (x);
> +}
> +
> +/* { dg-final { scan-assembler-times "vspltisw|vxor" 1 } } */
> +/* { dg-final { scan-assembler-times "vsubudm" 1 } } */
> +/* { dg-final { scan-assembler-times "vmaxsd" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-short.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-short.c
> new file mode 100644
> index 0000000..d312000
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-abs-short.c
> @@ -0,0 +1,18 @@
> +/* Verify that overloaded built-ins for vec_abs with short
> + inputs produce the right results. */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_altivec_ok } */
> +/* { dg-options "-maltivec -O2" } */
> +
> +#include <altivec.h>
> +
> +vector signed short
> +test3 (vector signed short x)
> +{
> + return vec_abs (x);
> +}
> +
> +/* { dg-final { scan-assembler-times "vspltisw|vxor" 1 } } */
> +/* { dg-final { scan-assembler-times "vsubuhm" 1 } } */
> +/* { dg-final { scan-assembler-times "vmaxsh" 1 } } */
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH, rs6000] Fold vector absolutes in GIMPLE
2017-05-29 8:39 ` Richard Biener
@ 2017-05-29 10:29 ` Segher Boessenkool
2017-05-29 11:55 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Segher Boessenkool @ 2017-05-29 10:29 UTC (permalink / raw)
To: Richard Biener; +Cc: will_schmidt, GCC Patches, David Edelsohn, Bill Schmidt
On Mon, May 29, 2017 at 10:32:18AM +0200, Richard Biener wrote:
> On Fri, May 26, 2017 at 7:19 PM, Will Schmidt <will_schmidt@vnet.ibm.com> wrote:
> > Add support for early expansion of vector absolute built-ins.
> >
> > Bootstraps currently running (p7,p8le,p8be).
> >
> > OK for trunk?
>
> 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."
http://openpowerfoundation.org/wp-content/uploads/resources/leabi-prd/
(appendix A; the PDF is easier to read).
Segher
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH, rs6000] Fold vector absolutes in GIMPLE
2017-05-29 10:29 ` Segher Boessenkool
@ 2017-05-29 11:55 ` Richard Biener
2017-05-29 12:23 ` Segher Boessenkool
0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2017-05-29 11:55 UTC (permalink / raw)
To: Segher Boessenkool
Cc: will_schmidt, GCC Patches, David Edelsohn, Bill Schmidt
On May 29, 2017 12:24:44 PM GMT+02:00, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>On Mon, May 29, 2017 at 10:32:18AM +0200, Richard Biener wrote:
>> On Fri, May 26, 2017 at 7:19 PM, Will Schmidt
><will_schmidt@vnet.ibm.com> wrote:
>> > Add support for early expansion of vector absolute built-ins.
>> >
>> > Bootstraps currently running (p7,p8le,p8be).
>> >
>> > OK for trunk?
>>
>> 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.
Richard.
>http://openpowerfoundation.org/wp-content/uploads/resources/leabi-prd/
>(appendix A; the PDF is easier to read).
>
>
>Segher
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH, rs6000] Fold vector absolutes in GIMPLE
2017-05-29 11:55 ` Richard Biener
@ 2017-05-29 12:23 ` Segher Boessenkool
2017-05-30 7:05 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Segher Boessenkool @ 2017-05-29 12:23 UTC (permalink / raw)
To: Richard Biener; +Cc: will_schmidt, GCC Patches, David Edelsohn, Bill Schmidt
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?
Segher
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH, rs6000] Fold vector absolutes in GIMPLE
2017-05-29 12:23 ` Segher Boessenkool
@ 2017-05-30 7:05 ` Richard Biener
2017-05-31 14:02 ` Will Schmidt
0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2017-05-30 7:05 UTC (permalink / raw)
To: Segher Boessenkool
Cc: will_schmidt, GCC Patches, David Edelsohn, Bill Schmidt
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.
Richard.
>
>
> Segher
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH, rs6000] Fold vector absolutes in GIMPLE
2017-05-30 7:05 ` Richard Biener
@ 2017-05-31 14:02 ` Will Schmidt
2017-05-31 14:04 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: Will Schmidt @ 2017-05-31 14:02 UTC (permalink / raw)
To: Richard Biener
Cc: Segher Boessenkool, GCC Patches, David Edelsohn, Bill Schmidt
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;
...
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH, rs6000] Fold vector absolutes in GIMPLE
2017-05-31 14:02 ` Will Schmidt
@ 2017-05-31 14:04 ` Richard Biener
2017-05-31 14:06 ` Ramana Radhakrishnan
0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2017-05-31 14:04 UTC (permalink / raw)
To: will_schmidt
Cc: Segher Boessenkool, GCC Patches, David Edelsohn, Bill Schmidt
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
>>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH, rs6000] Fold vector absolutes in GIMPLE
2017-05-31 14:04 ` Richard Biener
@ 2017-05-31 14:06 ` Ramana Radhakrishnan
0 siblings, 0 replies; 9+ messages in thread
From: Ramana Radhakrishnan @ 2017-05-31 14:06 UTC (permalink / raw)
To: Richard Biener
Cc: will_schmidt, Segher Boessenkool, GCC Patches, David Edelsohn,
Bill Schmidt
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
>>>
>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-05-31 14:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-26 17:19 [PATCH, rs6000] Fold vector absolutes in GIMPLE Will Schmidt
2017-05-29 8:39 ` Richard Biener
2017-05-29 10:29 ` Segher Boessenkool
2017-05-29 11:55 ` Richard Biener
2017-05-29 12:23 ` Segher Boessenkool
2017-05-30 7:05 ` Richard Biener
2017-05-31 14:02 ` Will Schmidt
2017-05-31 14:04 ` Richard Biener
2017-05-31 14:06 ` Ramana Radhakrishnan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).