* [PATCH, rs6000] Fix Vector long long subtype (PR96139)
@ 2020-09-02 2:00 will schmidt
2020-09-02 10:13 ` Segher Boessenkool
0 siblings, 1 reply; 7+ messages in thread
From: will schmidt @ 2020-09-02 2:00 UTC (permalink / raw)
To: gcc-patches, Segher Boessenkool, David Edelsohn
Hi,
This corrects an issue with the powerpc vector long long subtypes.
As reported by SjMunroe in PR96139. When building some code with -Wall
and attempting to print an element of a "long long vector" with a long long
printf format string, we will report a error because the vector sub-type
was improperly defined as int.
When defining a V2DI_type_node we use a TARGET_POWERPC64 ternary to
define the V2DI_type_node with "vector long" or "vector long long".
We also need to specify the proper sub-type when we define the type.
This has been successfully regtested on assorted power7,power8,
power8 targets. Another sniff test in the queue to verify a last
minute testcase tweak.
OK for trunk?
Thanks
-Will
PR target/96139
2020-09-01 Will Schmidt <will_schmidt@vnet.ibm.com>
gcc/Changelog:
* config/rs6000/rs6000-call.c (rs6000_init_builtin): Update V2DI_type_node
and unsigned_V2DI_type_node definitions.
gcc/testsuite/Changelog:
* testsuite/gcc.target/powerpc/pr96139-a.c: New test.
* testsuite/gcc.target/powerpc/pr96139-b.c: New test.
* testsuite/gcc.target/powerpc/pr96139-c.c: New test.
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index ff4c2537fada..354cac575cc6 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -12511,13 +12511,16 @@ rs6000_init_builtins (void)
if (TARGET_DEBUG_BUILTIN)
fprintf (stderr, "rs6000_init_builtins%s%s\n",
(TARGET_ALTIVEC) ? ", altivec" : "",
(TARGET_VSX) ? ", vsx" : "");
- V2DI_type_node = rs6000_vector_type (TARGET_POWERPC64 ? "__vector long"
- : "__vector long long",
- intDI_type_node, 2);
+ V2DI_type_node
+ = rs6000_vector_type (TARGET_POWERPC64
+ ? "__vector long" : "__vector long long",
+ TARGET_POWERPC64
+ ? long_long_integer_type_node : intDI_type_node,
+ 2);
V2DF_type_node = rs6000_vector_type ("__vector double", double_type_node, 2);
V4SI_type_node = rs6000_vector_type ("__vector signed int",
intSI_type_node, 4);
V4SF_type_node = rs6000_vector_type ("__vector float", float_type_node, 4);
V8HI_type_node = rs6000_vector_type ("__vector signed short",
@@ -12529,14 +12532,18 @@ rs6000_init_builtins (void)
unsigned_intQI_type_node, 16);
unsigned_V8HI_type_node = rs6000_vector_type ("__vector unsigned short",
unsigned_intHI_type_node, 8);
unsigned_V4SI_type_node = rs6000_vector_type ("__vector unsigned int",
unsigned_intSI_type_node, 4);
- unsigned_V2DI_type_node = rs6000_vector_type (TARGET_POWERPC64
- ? "__vector unsigned long"
- : "__vector unsigned long long",
- unsigned_intDI_type_node, 2);
+ unsigned_V2DI_type_node
+ = rs6000_vector_type (TARGET_POWERPC64
+ ? "__vector unsigned long"
+ : "__vector unsigned long long",
+ TARGET_POWERPC64
+ ? long_long_unsigned_type_node
+ : unsigned_intDI_type_node,
+ 2);
opaque_V4SI_type_node = build_opaque_vector_type (intSI_type_node, 4);
const_str_type_node
= build_pointer_type (build_qualified_type (char_type_node,
diff --git a/gcc/testsuite/gcc.target/powerpc/pr96139-a.c b/gcc/testsuite/gcc.target/powerpc/pr96139-a.c
new file mode 100644
index 000000000000..37820b1809ba
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr96139-a.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall -m32 " } */
+/* { dg-require-effective-target ilp32 } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+
+#include <stdio.h>
+#include <altivec.h>
+
+void
+try_printing_longlong_a (
+ __vector signed char cval,
+ __vector signed int ival,
+ __vector signed long long int llval,
+ int x, int y, int z)
+{
+ printf (" %016llx \n", llval[x]);
+ printf (" %016x \n", ival[z]);
+ printf (" %c \n", cval[y]);
+}
+
+void
+try_printing_unsigned_longlong_a (
+ __vector unsigned char cval,
+ __vector unsigned int ival,
+ __vector unsigned long long int llval,
+ int x, int y, int z)
+{
+ printf (" %016llx \n", llval[x]);
+ printf (" %016x \n", ival[z]);
+ printf (" %c \n", cval[y]);
+}
+
diff --git a/gcc/testsuite/gcc.target/powerpc/pr96139-b.c b/gcc/testsuite/gcc.target/powerpc/pr96139-b.c
new file mode 100644
index 000000000000..7fac6901790e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr96139-b.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall -m64 " } */
+/* { dg-require-effective-target lp64 } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+
+#include <stdio.h>
+#include <altivec.h>
+
+void
+try_printing_longlong_a (
+ __vector signed char cval,
+ __vector signed int ival,
+ __vector signed long long int llval,
+ int x, int y, int z)
+{
+ printf (" %016llx \n", llval[x]);
+ printf (" %016x \n", ival[z]);
+ printf (" %c \n", cval[y]);
+}
+
+
+void
+try_printing_unsigned_longlong_a (
+ __vector unsigned char cval,
+ __vector unsigned int ival,
+ __vector unsigned long long int llval,
+ int x, int y, int z)
+{
+ printf (" %016llx \n", llval[x]);
+ printf (" %016x \n", ival[z]);
+ printf (" %c \n", cval[y]);
+}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr96139-c.c b/gcc/testsuite/gcc.target/powerpc/pr96139-c.c
new file mode 100644
index 000000000000..2464b8da4f71
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr96139-c.c
@@ -0,0 +1,26 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -Wall" } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+
+/*
+ * Based on test created by sjmunroe for pr96139
+ */
+
+#include <stdio.h>
+#include <altivec.h>
+
+volatile vector long long llfoo;
+
+void
+print_v2xint64_b () {
+ printf (" %016llx \n", llfoo[0]);
+ printf (" %016llx \n", llfoo[1]);
+}
+
+int
+main() {
+llfoo[0]=12345678;
+llfoo[1]=34567890;
+print_v2xint64_b();
+return 0;
+}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, rs6000] Fix Vector long long subtype (PR96139)
2020-09-02 2:00 [PATCH, rs6000] Fix Vector long long subtype (PR96139) will schmidt
@ 2020-09-02 10:13 ` Segher Boessenkool
2020-09-03 15:37 ` will schmidt
0 siblings, 1 reply; 7+ messages in thread
From: Segher Boessenkool @ 2020-09-02 10:13 UTC (permalink / raw)
To: will schmidt; +Cc: gcc-patches, David Edelsohn
Hi Will,
On Tue, Sep 01, 2020 at 09:00:20PM -0500, will schmidt wrote:
> This corrects an issue with the powerpc vector long long subtypes.
> As reported by SjMunroe in PR96139. When building some code with -Wall
> and attempting to print an element of a "long long vector" with a long long
> printf format string, we will report a error because the vector sub-type
> was improperly defined as int.
>
> When defining a V2DI_type_node we use a TARGET_POWERPC64 ternary to
> define the V2DI_type_node with "vector long" or "vector long long".
> We also need to specify the proper sub-type when we define the type.
> - V2DI_type_node = rs6000_vector_type (TARGET_POWERPC64 ? "__vector long"
> - : "__vector long long",
> - intDI_type_node, 2);
> + V2DI_type_node
> + = rs6000_vector_type (TARGET_POWERPC64
> + ? "__vector long" : "__vector long long",
> + TARGET_POWERPC64
> + ? long_long_integer_type_node : intDI_type_node,
> + 2);
Can't you just use long_long_integer_type_node in all cases? Or, what
else is intDI_type_node for 32 bit?
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr96139-a.c
> @@ -0,0 +1,32 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall -m32 " } */
(trailing space, here and elsewhere -- not that it matters of course)
Segher
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, rs6000] Fix Vector long long subtype (PR96139)
2020-09-02 10:13 ` Segher Boessenkool
@ 2020-09-03 15:37 ` will schmidt
2020-09-03 18:09 ` Segher Boessenkool
0 siblings, 1 reply; 7+ messages in thread
From: will schmidt @ 2020-09-03 15:37 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches, David Edelsohn
On Wed, 2020-09-02 at 05:13 -0500, Segher Boessenkool wrote:
> Hi Will,
>
> On Tue, Sep 01, 2020 at 09:00:20PM -0500, will schmidt wrote:
> > This corrects an issue with the powerpc vector long long
> > subtypes.
> > As reported by SjMunroe in PR96139. When building some code with
> > -Wall
> > and attempting to print an element of a "long long vector" with a
> > long long
> > printf format string, we will report a error because the vector
> > sub-type
> > was improperly defined as int.
> >
> > When defining a V2DI_type_node we use a TARGET_POWERPC64 ternary to
> > define the V2DI_type_node with "vector long" or "vector long long".
> > We also need to specify the proper sub-type when we define the
> > type.
> > - V2DI_type_node = rs6000_vector_type (TARGET_POWERPC64 ?
> > "__vector long"
> > - : "__vector long long",
> > - intDI_type_node, 2);
> > + V2DI_type_node
> > + = rs6000_vector_type (TARGET_POWERPC64
> > + ? "__vector long" : "__vector long long",
> > + TARGET_POWERPC64
> > + ? long_long_integer_type_node :
> > intDI_type_node,
> > + 2);
>
> Can't you just use long_long_integer_type_node in all cases? Or,
> what
> else is intDI_type_node for 32 bit?
I'm not sure offhand. I'm tending to assume the use of intDI_type_node
is critical for some underlying reason. I'll give this a spin with
just long_long_integer_type_node and see what happens.
thanks,
>
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/pr96139-a.c
> > @@ -0,0 +1,32 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -Wall -m32 " } */
>
> (trailing space, here and elsewhere -- not that it matters of course)
yup, will fix. thanks for the review.
-Will
>
>
> Segher
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, rs6000] Fix Vector long long subtype (PR96139)
2020-09-03 15:37 ` will schmidt
@ 2020-09-03 18:09 ` Segher Boessenkool
2020-09-04 6:55 ` Richard Biener
0 siblings, 1 reply; 7+ messages in thread
From: Segher Boessenkool @ 2020-09-03 18:09 UTC (permalink / raw)
To: will schmidt; +Cc: gcc-patches, David Edelsohn
Hi!
On Thu, Sep 03, 2020 at 10:37:33AM -0500, will schmidt wrote:
> On Wed, 2020-09-02 at 05:13 -0500, Segher Boessenkool wrote:
> > On Tue, Sep 01, 2020 at 09:00:20PM -0500, will schmidt wrote:
> > > This corrects an issue with the powerpc vector long long
> > > subtypes.
> > > As reported by SjMunroe in PR96139. When building some code with
> > > -Wall
> > > and attempting to print an element of a "long long vector" with a
> > > long long
> > > printf format string, we will report a error because the vector
> > > sub-type
> > > was improperly defined as int.
> > >
> > > When defining a V2DI_type_node we use a TARGET_POWERPC64 ternary to
> > > define the V2DI_type_node with "vector long" or "vector long long".
> > > We also need to specify the proper sub-type when we define the
> > > type.
> > > - V2DI_type_node = rs6000_vector_type (TARGET_POWERPC64 ?
> > > "__vector long"
> > > - : "__vector long long",
> > > - intDI_type_node, 2);
> > > + V2DI_type_node
> > > + = rs6000_vector_type (TARGET_POWERPC64
> > > + ? "__vector long" : "__vector long long",
> > > + TARGET_POWERPC64
> > > + ? long_long_integer_type_node :
> > > intDI_type_node,
> > > + 2);
> >
> > Can't you just use long_long_integer_type_node in all cases? Or,
> > what
> > else is intDI_type_node for 32 bit?
>
> I'm not sure offhand. I'm tending to assume the use of intDI_type_node
> is critical for some underlying reason. I'll give this a spin with
> just long_long_integer_type_node and see what happens.
If that works, that is okay for trunk (and all backports you want). If
not, just use what you sent.
Thanks!
Segher
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, rs6000] Fix Vector long long subtype (PR96139)
2020-09-03 18:09 ` Segher Boessenkool
@ 2020-09-04 6:55 ` Richard Biener
2020-09-04 8:47 ` Segher Boessenkool
0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2020-09-04 6:55 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: will schmidt, GCC Patches, David Edelsohn
On Thu, Sep 3, 2020 at 8:10 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> Hi!
>
> On Thu, Sep 03, 2020 at 10:37:33AM -0500, will schmidt wrote:
> > On Wed, 2020-09-02 at 05:13 -0500, Segher Boessenkool wrote:
> > > On Tue, Sep 01, 2020 at 09:00:20PM -0500, will schmidt wrote:
> > > > This corrects an issue with the powerpc vector long long
> > > > subtypes.
> > > > As reported by SjMunroe in PR96139. When building some code with
> > > > -Wall
> > > > and attempting to print an element of a "long long vector" with a
> > > > long long
> > > > printf format string, we will report a error because the vector
> > > > sub-type
> > > > was improperly defined as int.
> > > >
> > > > When defining a V2DI_type_node we use a TARGET_POWERPC64 ternary to
> > > > define the V2DI_type_node with "vector long" or "vector long long".
> > > > We also need to specify the proper sub-type when we define the
> > > > type.
> > > > - V2DI_type_node = rs6000_vector_type (TARGET_POWERPC64 ?
> > > > "__vector long"
> > > > - : "__vector long long",
> > > > - intDI_type_node, 2);
> > > > + V2DI_type_node
> > > > + = rs6000_vector_type (TARGET_POWERPC64
> > > > + ? "__vector long" : "__vector long long",
> > > > + TARGET_POWERPC64
> > > > + ? long_long_integer_type_node :
> > > > intDI_type_node,
> > > > + 2);
> > >
> > > Can't you just use long_long_integer_type_node in all cases? Or,
> > > what
> > > else is intDI_type_node for 32 bit?
> >
> > I'm not sure offhand. I'm tending to assume the use of intDI_type_node
> > is critical for some underlying reason. I'll give this a spin with
> > just long_long_integer_type_node and see what happens.
>
> If that works, that is okay for trunk (and all backports you want). If
> not, just use what you sent.
Beware of alias issues! 'long' and 'long long' are distinct types even if
they have the same size and long * aliases with vector<long> * but not
with vector<long long> *.
So if the user writes 'vector long' you should use the appropriate component
type. If the user writes 'vector intDI_type' you should use intDI_type.
Richard.
> Thanks!
>
>
> Segher
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, rs6000] Fix Vector long long subtype (PR96139)
2020-09-04 6:55 ` Richard Biener
@ 2020-09-04 8:47 ` Segher Boessenkool
2020-09-04 14:47 ` will schmidt
0 siblings, 1 reply; 7+ messages in thread
From: Segher Boessenkool @ 2020-09-04 8:47 UTC (permalink / raw)
To: Richard Biener; +Cc: will schmidt, GCC Patches, David Edelsohn
Hi!
On Fri, Sep 04, 2020 at 08:55:43AM +0200, Richard Biener wrote:
> On Thu, Sep 3, 2020 at 8:10 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Thu, Sep 03, 2020 at 10:37:33AM -0500, will schmidt wrote:
> > > On Wed, 2020-09-02 at 05:13 -0500, Segher Boessenkool wrote:
> > > > On Tue, Sep 01, 2020 at 09:00:20PM -0500, will schmidt wrote:
> > > > > - V2DI_type_node = rs6000_vector_type (TARGET_POWERPC64 ?
> > > > > "__vector long"
> > > > > - : "__vector long long",
> > > > > - intDI_type_node, 2);
> > > > > + V2DI_type_node
> > > > > + = rs6000_vector_type (TARGET_POWERPC64
> > > > > + ? "__vector long" : "__vector long long",
> > > > > + TARGET_POWERPC64
> > > > > + ? long_long_integer_type_node :
> > > > > intDI_type_node,
> > > > > + 2);
> > > >
> > > > Can't you just use long_long_integer_type_node in all cases? Or,
> > > > what
> > > > else is intDI_type_node for 32 bit?
> > >
> > > I'm not sure offhand. I'm tending to assume the use of intDI_type_node
> > > is critical for some underlying reason. I'll give this a spin with
> > > just long_long_integer_type_node and see what happens.
> >
> > If that works, that is okay for trunk (and all backports you want). If
> > not, just use what you sent.
>
> Beware of alias issues! 'long' and 'long long' are distinct types even if
> they have the same size and long * aliases with vector<long> * but not
> with vector<long long> *.
That is what this patch is all about, so I do beware (right now ;-) ),
don't worry :-)
> So if the user writes 'vector long' you should use the appropriate component
> type. If the user writes 'vector intDI_type' you should use intDI_type.
Yes. But intDI is not something the user can write, it is an internal
name, and for 32 bit it is just long long int. We use intDI if
something is long on 64 bit and long long on 32 bit, but here we always
want long long, so we can just as well just write that :-)
intDI_type_node = make_or_reuse_type (GET_MODE_BITSIZE (DImode), 0);
Segher
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, rs6000] Fix Vector long long subtype (PR96139)
2020-09-04 8:47 ` Segher Boessenkool
@ 2020-09-04 14:47 ` will schmidt
0 siblings, 0 replies; 7+ messages in thread
From: will schmidt @ 2020-09-04 14:47 UTC (permalink / raw)
To: Segher Boessenkool, Richard Biener; +Cc: GCC Patches, David Edelsohn
On Fri, 2020-09-04 at 03:47 -0500, Segher Boessenkool wrote:
> Hi!
>
> On Fri, Sep 04, 2020 at 08:55:43AM +0200, Richard Biener wrote:
> > On Thu, Sep 3, 2020 at 8:10 PM Segher Boessenkool
> > <segher@kernel.crashing.org> wrote:
> > > On Thu, Sep 03, 2020 at 10:37:33AM -0500, will schmidt wrote:
> > > > On Wed, 2020-09-02 at 05:13 -0500, Segher Boessenkool wrote:
> > > > > On Tue, Sep 01, 2020 at 09:00:20PM -0500, will schmidt wrote:
> > > > > > - V2DI_type_node = rs6000_vector_type (TARGET_POWERPC64 ?
> > > > > > "__vector long"
> > > > > > - : "__vector long long",
> > > > > > - intDI_type_node, 2);
> > > > > > + V2DI_type_node
> > > > > > + = rs6000_vector_type (TARGET_POWERPC64
> > > > > > + ? "__vector long" : "__vector long
> > > > > > long",
> > > > > > + TARGET_POWERPC64
> > > > > > + ? long_long_integer_type_node :
> > > > > > intDI_type_node,
> > > > > > + 2);
> > > > >
> > > > > Can't you just use long_long_integer_type_node in all
> > > > > cases? Or,
> > > > > what
> > > > > else is intDI_type_node for 32 bit?
> > > >
> > > > I'm not sure offhand. I'm tending to assume the use of
> > > > intDI_type_node
> > > > is critical for some underlying reason. I'll give this a
> > > > spin with
> > > > just long_long_integer_type_node and see what happens.
> > >
> > > If that works, that is okay for trunk (and all backports you
> > > want). If
> > > not, just use what you sent.
> >
> > Beware of alias issues! 'long' and 'long long' are distinct types
> > even if
> > they have the same size and long * aliases with vector<long> * but
> > not
> > with vector<long long> *.
>
> That is what this patch is all about, so I do beware (right now ;-)
> ),
> don't worry :-)
>
> > So if the user writes 'vector long' you should use the appropriate
> > component
> > type. If the user writes 'vector intDI_type' you should use
> > intDI_type.
>
> Yes. But intDI is not something the user can write, it is an
> internal
> name, and for 32 bit it is just long long int. We use intDI if
> something is long on 64 bit and long long on 32 bit, but here we
> always
> want long long, so we can just as well just write that :-)
>
> intDI_type_node = make_or_reuse_type (GET_MODE_BITSIZE (DImode),
> 0);
Thanks for the additional scrutiny and discussion.. :-)
My regtests with that change passed cleanly, so it was committed
yesterday.
Thanks,
-Will
>
>
> Segher
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-09-04 14:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02 2:00 [PATCH, rs6000] Fix Vector long long subtype (PR96139) will schmidt
2020-09-02 10:13 ` Segher Boessenkool
2020-09-03 15:37 ` will schmidt
2020-09-03 18:09 ` Segher Boessenkool
2020-09-04 6:55 ` Richard Biener
2020-09-04 8:47 ` Segher Boessenkool
2020-09-04 14:47 ` will schmidt
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).