[PATCH] [RS6000] Change maddld match_operand from DI to GPR

[Kewen.Lin]

Hi Lijia,

on 2019/6/24 脧脗脦莽2:00, Li Jia He wrote:
> Hi,
> 
> From PowerPC ISA3.0, the description of `maddld RT, RA.RB, RC` is as follows:
> 64-bit RA and RB are multiplied and then the RC is signed extend to 128 bits,
> and add them together.
> 
> We only apply it to 64-bit mode (DI) when implementing maddld.  However, if we
> can guarantee that the result of the maddld operation will be limited to 32-bit
> mode (SI), we can still apply it to 32-bit mode (SI).
> 
> The regression testing for the patch was done on GCC mainline on
> 
>     powerpc64le-unknown-linux-gnu (Power 9 LE)
> 
> with no regressions.  Is it OK for trunk ?
> 
> Thanks,
> Lijia He
> 
> gcc/ChangeLog
> 2019-06-24  Li Jia He  <helijia@linux.ibm.com>
> 
> 	* config/rs6000/rs6000.h (TARGET_MADDLD): Remove the restriction of
> 	TARGET_POWERPC64.
> 	* config/rs6000/rs6000.md (maddld): Change maddld match_operand from DI
> 	to GPR.
> 
> gcc/testsuite/ChangeLog
> 
> 2019-06-24  Li Jia He  <helijia@linux.ibm.com>
> 
> 	* gcc.target/powerpc/maddld-1.c: New testcase.
> ---
>  gcc/config/rs6000/rs6000.h                  |  2 +-
>  gcc/config/rs6000/rs6000.md                 | 10 +++++-----
>  gcc/testsuite/gcc.target/powerpc/maddld-1.c | 19 +++++++++++++++++++
>  3 files changed, 25 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/maddld-1.c
> 
> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 34fa36b6ed9..f83f19afbba 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -453,7 +453,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 && TARGET_POWERPC64)
> +#define TARGET_MADDLD	TARGET_MODULO
>  

IMHO, I don't think this removal of TARGET_POWERPC64 is reasonable.
As ISA V3.0, the description of this insn maddld is:
GPR[RT].dword[0] 隆没 Chop(result, 64)

It assumes the GPR has dword, it's a 64-bit specific insn, right?
Your change relaxes it to be adopted on 32-bit.
Although it's fine for powerpc LE since it's always 64-bit, it will
have problems for power9 32bit like AIX?

>  #define TARGET_XSCVDPSPN	(TARGET_DIRECT_MOVE || TARGET_P8_VECTOR)
>  #define TARGET_XSCVSPDPN	(TARGET_DIRECT_MOVE || TARGET_P8_VECTOR)
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 47cbba89443..9122b29e99b 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -3057,11 +3057,11 @@
>    DONE;
>  })
>  
> -(define_insn "*maddld4"
> -  [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
> -	(plus:DI (mult:DI (match_operand:DI 1 "gpc_reg_operand" "r")
> -			  (match_operand:DI 2 "gpc_reg_operand" "r"))
> -		 (match_operand:DI 3 "gpc_reg_operand" "r")))]
> +(define_insn "*maddld<mode>4"
> +  [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
> +	(plus:GPR (mult:GPR (match_operand:GPR 1 "gpc_reg_operand" "r")
> +			    (match_operand:GPR 2 "gpc_reg_operand" "r"))
> +		  (match_operand:GPR 3 "gpc_reg_operand" "r")))]
>    "TARGET_MADDLD"
>    "maddld %0,%1,%2,%3"
>    [(set_attr "type" "mul")])
> diff --git a/gcc/testsuite/gcc.target/powerpc/maddld-1.c b/gcc/testsuite/gcc.target/powerpc/maddld-1.c
> new file mode 100644
> index 00000000000..06f5f5774d4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/maddld-1.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */


The above target requirement seems useless? since you have
below one which is more specific.


Thanks,
Kewen

> +/* { dg-require-effective-target powerpc_p9modulo_ok } */
> +/* { dg-options "-mdejagnu-cpu=power9 -O2" } */
> +

>