diff mbox series

[rs6000] TARGET_MADDLD should include TARGET_POWERPC64

Message ID eae863c9-661b-328d-3bbe-66b8e97c8fd1@linux.ibm.com
State New
Headers
Series [rs6000] TARGET_MADDLD should include TARGET_POWERPC64 |

Commit Message

HAO CHEN GUI Aug. 3, 2022, 8:24 a.m. UTC 
  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

  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.

gcc/testsuite/
	* gcc.target/powerpc/maddld-1.c: Modify target requirement to compile
	it on the target which supports 64 bit instructions.


patch.diff

Comments

Kewen.Lin Aug. 3, 2022, 9:23 a.m. UTC | #1 
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
Segher Boessenkool Aug. 3, 2022, 4:54 p.m. UTC | #2 
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
HAO CHEN GUI Aug. 4, 2022, 3:17 a.m. UTC | #3 
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
Segher Boessenkool Aug. 4, 2022, 5:24 p.m. UTC | #4 
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 mbox series

Patch

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