Message ID | eae863c9-661b-328d-3bbe-66b8e97c8fd1@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | [rs6000] TARGET_MADDLD should include TARGET_POWERPC64 | expand |
Hi Haochen, on 2022/8/3 16:24, HAO CHEN GUI wrote: > Hi, > This patch changes the definition of TARGET_MADDLD and includes > TARGET_POWERPC64, since maddld is a 64 bit instruction. > > maddld-1.c now checks "has_arch_ppc64". It depends on a patch which fixes > empty TU problem. > https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598744.html > aha, I'm going to push it if Segher doesn't have further comments. :) > Bootstrapped and tested on powerpc64-linux BE and LE with no regressions. > Is this okay for trunk? Any recommendations? Thanks a lot. > > ChangeLog > 2022-08-03 Haochen Gui <guihaoc@linux.ibm.com> > > gcc/ > * config/rs6000/rs6000.h (TARGET_MADDLD): Define. May be ": Adjust." or ": Adjust with TARGET_POWERPC64.". > > gcc/testsuite/ > * gcc.target/powerpc/maddld-1.c: Modify target requirement to compile > it on the target which supports 64 bit instructions. > May be ": Add effective target has_arch_ppc64." OK with these nits adjusted or not. BR, Kewen > > patch.diff > diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h > index 7d04556304a..2f15451fd8b 100644 > --- a/gcc/config/rs6000/rs6000.h > +++ b/gcc/config/rs6000/rs6000.h > @@ -466,7 +466,7 @@ extern int rs6000_vector_align[]; > #define TARGET_FCTIWUZ TARGET_POPCNTD > #define TARGET_CTZ TARGET_MODULO > #define TARGET_EXTSWSLI (TARGET_MODULO && TARGET_POWERPC64) > -#define TARGET_MADDLD TARGET_MODULO > +#define TARGET_MADDLD (TARGET_MODULO && TARGET_POWERPC64) > > #define TARGET_XSCVDPSPN (TARGET_DIRECT_MOVE || TARGET_P8_VECTOR) > #define TARGET_XSCVSPDPN (TARGET_DIRECT_MOVE || TARGET_P8_VECTOR) > diff --git a/gcc/testsuite/gcc.target/powerpc/maddld-1.c b/gcc/testsuite/gcc.target/powerpc/maddld-1.c > index 4edecf1c86d..0a53658e058 100644 > --- a/gcc/testsuite/gcc.target/powerpc/maddld-1.c > +++ b/gcc/testsuite/gcc.target/powerpc/maddld-1.c > @@ -1,4 +1,4 @@ > -/* { dg-do compile } */ > +/* { dg-do compile { target { has_arch_ppc64 } } } */ > /* { dg-options "-mdejagnu-cpu=power9 -O2" } */ > > /* This file tests the maddld instruction can be used in SI mode
Hi! On Wed, Aug 03, 2022 at 04:24:15PM +0800, HAO CHEN GUI wrote: > This patch changes the definition of TARGET_MADDLD and includes > TARGET_POWERPC64, since maddld is a 64 bit instruction. Hrm. But the maddld insn is useful for SImode as well, in 32-bit mode, it is just its name that is a bit confusing then. Sorry for confusing things :-( Add a test for SImode maddld as well? Please fix things up once again and resend? Sorry again! Segher
Hi Segher, On 4/8/2022 上午 12:54, Segher Boessenkool wrote: > Hrm. But the maddld insn is useful for SImode as well, in 32-bit mode, > it is just its name that is a bit confusing then. Sorry for confusing > things :-( > > Add a test for SImode maddld as well? Thanks for your comments. Just want to double confirm that a maddld test case with "-m32" and "-mpowerpc64" is needed. As far as I understand, maddld is a 64-bit instruction and it should be used with "-mpowerpc64" in a 32-bit register environment. Thanks again Gui Haochen
Hi! On Thu, Aug 04, 2022 at 11:17:48AM +0800, HAO CHEN GUI wrote: > On 4/8/2022 上午 12:54, Segher Boessenkool wrote: > > Hrm. But the maddld insn is useful for SImode as well, in 32-bit mode, > > it is just its name that is a bit confusing then. Sorry for confusing > > things :-( > > > > Add a test for SImode maddld as well? > > Thanks for your comments. > > Just want to double confirm that a maddld test case with "-m32" and > "-mpowerpc64" is needed. As far as I understand, maddld is a 64-bit > instruction and it should be used with "-mpowerpc64" in a 32-bit register > environment. Nope. The instruction is fine in pure 32 bit as well. Almost all instructions actually work on 64 bits, but for many (including this maddld) the low 32 bits of the result make sense on their own, as a 32-bit operation done on the low 32 bits of the input registers. We have (define_mode_iterator GPR [SI (DI "TARGET_POWERPC64")]) so that :DI will not be used for plain -m32 compilations, but it can (and will, and should) be used for -m32 -mpowerpc64, and :SI can be used for -m32 in every case. Instructions that look at the top 32 bits of a GPR need an explicit TARGET_POWERPC64. Segher
diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h index 7d04556304a..2f15451fd8b 100644 --- a/gcc/config/rs6000/rs6000.h +++ b/gcc/config/rs6000/rs6000.h @@ -466,7 +466,7 @@ extern int rs6000_vector_align[]; #define TARGET_FCTIWUZ TARGET_POPCNTD #define TARGET_CTZ TARGET_MODULO #define TARGET_EXTSWSLI (TARGET_MODULO && TARGET_POWERPC64) -#define TARGET_MADDLD TARGET_MODULO +#define TARGET_MADDLD (TARGET_MODULO && TARGET_POWERPC64) #define TARGET_XSCVDPSPN (TARGET_DIRECT_MOVE || TARGET_P8_VECTOR) #define TARGET_XSCVSPDPN (TARGET_DIRECT_MOVE || TARGET_P8_VECTOR) diff --git a/gcc/testsuite/gcc.target/powerpc/maddld-1.c b/gcc/testsuite/gcc.target/powerpc/maddld-1.c index 4edecf1c86d..0a53658e058 100644 --- a/gcc/testsuite/gcc.target/powerpc/maddld-1.c +++ b/gcc/testsuite/gcc.target/powerpc/maddld-1.c @@ -1,4 +1,4 @@ -/* { dg-do compile } */ +/* { dg-do compile { target { has_arch_ppc64 } } } */ /* { dg-options "-mdejagnu-cpu=power9 -O2" } */ /* This file tests the maddld instruction can be used in SI mode