* [PATCH] [RS6000] Change maddld match_operand from DI to GPR
@ 2019-06-24 6:00 Li Jia He
2019-06-24 6:45 ` Kewen.Lin
2019-06-24 7:39 ` Segher Boessenkool
0 siblings, 2 replies; 10+ messages in thread
From: Li Jia He @ 2019-06-24 6:00 UTC (permalink / raw)
To: gcc-patches; +Cc: segher, wschmidt, Li Jia He
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
#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*-*-* } } } */
+/* { dg-require-effective-target powerpc_p9modulo_ok } */
+/* { dg-options "-mdejagnu-cpu=power9 -O2" } */
+
+int
+s_madd (int a, int b, int c)
+{
+ return (a * b) + c;
+}
+
+unsigned int
+u_madd (unsigned int a, unsigned int b, unsigned int c)
+{
+ return (a * b) + c;
+}
+
+/* { dg-final { scan-assembler-times "maddld " 2 } } */
+/* { dg-final { scan-assembler-not "mulld " } } */
+/* { dg-final { scan-assembler-not "add " } } */
--
2.17.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RS6000] Change maddld match_operand from DI to GPR
2019-06-24 6:00 [PATCH] [RS6000] Change maddld match_operand from DI to GPR Li Jia He
@ 2019-06-24 6:45 ` Kewen.Lin
2019-06-24 7:19 ` Segher Boessenkool
2019-06-24 7:39 ` Segher Boessenkool
1 sibling, 1 reply; 10+ messages in thread
From: Kewen.Lin @ 2019-06-24 6:45 UTC (permalink / raw)
To: Li Jia He; +Cc: gcc-patches, segher, wschmidt
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" } */
> +
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RS6000] Change maddld match_operand from DI to GPR
2019-06-24 6:45 ` Kewen.Lin
@ 2019-06-24 7:19 ` Segher Boessenkool
2019-06-24 7:43 ` Kewen.Lin
0 siblings, 1 reply; 10+ messages in thread
From: Segher Boessenkool @ 2019-06-24 7:19 UTC (permalink / raw)
To: Kewen.Lin; +Cc: Li Jia He, gcc-patches, wschmidt
On Mon, Jun 24, 2019 at 02:45:09PM +0800, Kewen.Lin wrote:
> on 2019/6/24 ä¸å2:00, Li Jia He wrote:
> > -#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?
Hi Kewen,
Newer ISAs require 64-bit to be implemented. There are no optional
64-bit categories anymore. Since this instruction is enabled for P9
(ISA 3.0) only (that's the TARGET_MODULO), it's fine.
What you are saying is quite true for older CPUs/ISAs though: there you
have to make sure you are targetting a CPU that supports the 64-bit
categories, before using any 64-bit insns.
But those days are gone :-)
Segher
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RS6000] Change maddld match_operand from DI to GPR
2019-06-24 6:00 [PATCH] [RS6000] Change maddld match_operand from DI to GPR Li Jia He
2019-06-24 6:45 ` Kewen.Lin
@ 2019-06-24 7:39 ` Segher Boessenkool
2019-06-26 5:07 ` Li Jia He
1 sibling, 1 reply; 10+ messages in thread
From: Segher Boessenkool @ 2019-06-24 7:39 UTC (permalink / raw)
To: Li Jia He; +Cc: gcc-patches, wschmidt
Hi Lijia,
On Mon, Jun 24, 2019 at 01:00:05AM -0500, Li Jia He wrote:
> >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).
Great :-) Just some testcase comments:
> 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*-*-* } } } */
powerpc* is the default in gcc.target/powerpc, so you can leave it out:
/* { dg-do compile } */
(and that is default itself, but it is good documentation for the target
tests, many of those are run tests).
> +/* { dg-require-effective-target powerpc_p9modulo_ok } */
You don't need this line, it tests if the assembler supports p9.
> +/* { dg-final { scan-assembler-times "maddld " 2 } } */
> +/* { dg-final { scan-assembler-not "mulld " } } */
> +/* { dg-final { scan-assembler-not "add " } } */
You can easier write this using \m and \M, a bit more exact even:
/* { dg-final { scan-assembler-times {\mmaddld\M} 2 } } */
/* { dg-final { scan-assembler-not {\mmul} } } */
/* { dg-final { scan-assembler-not {\madd} } } */
Which allows only the exact mnemonic "maddld", and disallows anything
starting with "mul" or "add".
Okay for trunk, with the testcase improvements please. Thanks!
Segher
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RS6000] Change maddld match_operand from DI to GPR
2019-06-24 7:19 ` Segher Boessenkool
@ 2019-06-24 7:43 ` Kewen.Lin
2019-06-24 7:49 ` Kewen.Lin
2019-06-24 8:02 ` Segher Boessenkool
0 siblings, 2 replies; 10+ messages in thread
From: Kewen.Lin @ 2019-06-24 7:43 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Li Jia He, gcc-patches, wschmidt
on 2019/6/24 ä¸å3:19, Segher Boessenkool wrote:
> On Mon, Jun 24, 2019 at 02:45:09PM +0800, Kewen.Lin wrote:
>> on 2019/6/24 ä¸å2:00, Li Jia He wrote:
>>> -#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?
>
> Hi Kewen,
>
> Newer ISAs require 64-bit to be implemented. There are no optional
> 64-bit categories anymore. Since this instruction is enabled for P9
> (ISA 3.0) only (that's the TARGET_MODULO), it's fine.
>
> What you are saying is quite true for older CPUs/ISAs though: there you
> have to make sure you are targetting a CPU that supports the 64-bit
> categories, before using any 64-bit insns.
>
> But those days are gone :-)
>
Hi Segher,
Good to know that, thanks a lot for the information! It's fine then.
It sounds like we can have a clean up for some others like
TARGET_EXTSWSLI. :)
Thanks,
Kewen
>
> Segher
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RS6000] Change maddld match_operand from DI to GPR
2019-06-24 7:43 ` Kewen.Lin
@ 2019-06-24 7:49 ` Kewen.Lin
2019-06-24 8:16 ` Segher Boessenkool
2019-06-24 8:02 ` Segher Boessenkool
1 sibling, 1 reply; 10+ messages in thread
From: Kewen.Lin @ 2019-06-24 7:49 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Li Jia He, gcc-patches, wschmidt
on 2019/6/24 ä¸å3:43, Kewen.Lin wrote:
> on 2019/6/24 ä¸å3:19, Segher Boessenkool wrote:
>> On Mon, Jun 24, 2019 at 02:45:09PM +0800, Kewen.Lin wrote:
>>> on 2019/6/24 ä¸å2:00, Li Jia He wrote:
>>>> -#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?
>>
>> Hi Kewen,
>>
>> Newer ISAs require 64-bit to be implemented. There are no optional
>> 64-bit categories anymore. Since this instruction is enabled for P9
>> (ISA 3.0) only (that's the TARGET_MODULO), it's fine.
>>
>> What you are saying is quite true for older CPUs/ISAs though: there you
>> have to make sure you are targetting a CPU that supports the 64-bit
>> categories, before using any 64-bit insns.
>>
>> But those days are gone :-)
>>
>
> Hi Segher,
>
> Good to know that, thanks a lot for the information! It's fine then.
>
> It sounds like we can have a clean up for some others like
> TARGET_EXTSWSLI. :)
>
Sorry, maybe not, it's not similar to maddld for 32bit operations.
Thanks,
Kewen
>
> Thanks,
> Kewen
>
>>
>> Segher
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RS6000] Change maddld match_operand from DI to GPR
2019-06-24 7:43 ` Kewen.Lin
2019-06-24 7:49 ` Kewen.Lin
@ 2019-06-24 8:02 ` Segher Boessenkool
2019-06-24 8:10 ` Kewen.Lin
1 sibling, 1 reply; 10+ messages in thread
From: Segher Boessenkool @ 2019-06-24 8:02 UTC (permalink / raw)
To: Kewen.Lin; +Cc: Li Jia He, gcc-patches, wschmidt
Hi Kewen,
On Mon, Jun 24, 2019 at 03:43:26PM +0800, Kewen.Lin wrote:
> on 2019/6/24 ä¸å3:19, Segher Boessenkool wrote:
> > Newer ISAs require 64-bit to be implemented. There are no optional
> > 64-bit categories anymore. Since this instruction is enabled for P9
> > (ISA 3.0) only (that's the TARGET_MODULO), it's fine.
> >
> > What you are saying is quite true for older CPUs/ISAs though: there you
> > have to make sure you are targetting a CPU that supports the 64-bit
> > categories, before using any 64-bit insns.
> >
> > But those days are gone :-)
>
> Good to know that, thanks a lot for the information! It's fine then.
>
> It sounds like we can have a clean up for some others like
> TARGET_EXTSWSLI. :)
Yes, but be careful there! The insn patterns for this use DImode, which
does not mean the same thing without -mpowerpc64 (it's a register pair
then, not what you want).
And it doesn't make much sense to allow this for SImode as well (using
GPR, perhaps), because the insn just is a shift left for SImode, and we
already have shift left instructions.
So we might want to just directly say "TARGET_MODULO && TARGET_POWERPC64"
in those patterns (TARGET_MODULO is a funny way of saying "p9 or later").
Segher
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RS6000] Change maddld match_operand from DI to GPR
2019-06-24 8:02 ` Segher Boessenkool
@ 2019-06-24 8:10 ` Kewen.Lin
0 siblings, 0 replies; 10+ messages in thread
From: Kewen.Lin @ 2019-06-24 8:10 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Li Jia He, gcc-patches, wschmidt
Hi Segher,
on 2019/6/24 ä¸å4:02, Segher Boessenkool wrote:
> Hi Kewen,
>
> On Mon, Jun 24, 2019 at 03:43:26PM +0800, Kewen.Lin wrote:
>> on 2019/6/24 ä¸å3:19, Segher Boessenkool wrote:
>>> Newer ISAs require 64-bit to be implemented. There are no optional
>>> 64-bit categories anymore. Since this instruction is enabled for P9
>>> (ISA 3.0) only (that's the TARGET_MODULO), it's fine.
>>>
>>> What you are saying is quite true for older CPUs/ISAs though: there you
>>> have to make sure you are targetting a CPU that supports the 64-bit
>>> categories, before using any 64-bit insns.
>>>
>>> But those days are gone :-)
>>
>> Good to know that, thanks a lot for the information! It's fine then.
>>
>> It sounds like we can have a clean up for some others like
>> TARGET_EXTSWSLI. :)
>
> Yes, but be careful there! The insn patterns for this use DImode, which
> does not mean the same thing without -mpowerpc64 (it's a register pair
> then, not what you want).
>
> And it doesn't make much sense to allow this for SImode as well (using
> GPR, perhaps), because the insn just is a shift left for SImode, and we
> already have shift left instructions.
>
> So we might want to just directly say "TARGET_MODULO && TARGET_POWERPC64"
> in those patterns (TARGET_MODULO is a funny way of saying "p9 or later").
>
Thanks for further clarification! Yes, I agree with you. I just noticed
that extswsli isn't like maddld and not suitable for SImode.
Thanks,
Kewen
>
> Segher
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RS6000] Change maddld match_operand from DI to GPR
2019-06-24 7:49 ` Kewen.Lin
@ 2019-06-24 8:16 ` Segher Boessenkool
0 siblings, 0 replies; 10+ messages in thread
From: Segher Boessenkool @ 2019-06-24 8:16 UTC (permalink / raw)
To: Kewen.Lin; +Cc: Li Jia He, gcc-patches, wschmidt
On Mon, Jun 24, 2019 at 03:49:35PM +0800, Kewen.Lin wrote:
> > It sounds like we can have a clean up for some others like
> > TARGET_EXTSWSLI. :)
>
> Sorry, maybe not, it's not similar to maddld for 32bit operations.
Hey, it currently is
(define_insn_and_split "ashdi3_extswsli"
[(set (match_operand:DI 0 "gpc_reg_operand" "=r,r")
(ashift:DI
(sign_extend:DI (match_operand:SI 1 "reg_or_mem_operand" "r,m"))
(match_operand:DI 2 "u6bit_cint_operand" "n,n")))]
so you could just do
(define_insn_and_split "ashdi3_extswsli"
[(set (match_operand:GPR 0 "gpc_reg_operand" "=r,r")
(ashift:GPR
(sign_extend:GPR (match_operand:SI 1 "reg_or_mem_operand" "r,m"))
(match_operand:GPR 2 "u6bit_cint_operand" "n,n")))]
and that will work, just generate insn patterns that will never match for SI.
But you can also do
(define_insn_and_split "ashl<mode>3_extswsli"
[(set (match_operand:EXTSI 0 "gpc_reg_operand" "=r,r")
(ashift:EXTSI
(sign_extend:EXTSI (match_operand:SI 1 "reg_or_mem_operand" "r,m"))
(match_operand:EXTSI 2 "u6bit_cint_operand" "n,n")))]
and that should work fine, without needing any explicit TARGET_POWERPC64.
But now you need to adjust direct callers of this pattern (which probably
do exist, it is a named pattern (i.e. without *) for a reason ;-)
Segher
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RS6000] Change maddld match_operand from DI to GPR
2019-06-24 7:39 ` Segher Boessenkool
@ 2019-06-26 5:07 ` Li Jia He
0 siblings, 0 replies; 10+ messages in thread
From: Li Jia He @ 2019-06-26 5:07 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches, wschmidt
On 2019/6/24 3:38 PM, Segher Boessenkool wrote:
> Hi Lijia,
>
> On Mon, Jun 24, 2019 at 01:00:05AM -0500, Li Jia He wrote:
>> >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).
>
> Great :-) Just some testcase comments:
>
>> 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*-*-* } } } */
>
> powerpc* is the default in gcc.target/powerpc, so you can leave it out:
>
> /* { dg-do compile } */
>
> (and that is default itself, but it is good documentation for the target
> tests, many of those are run tests).
>
>> +/* { dg-require-effective-target powerpc_p9modulo_ok } */
>
> You don't need this line, it tests if the assembler supports p9.
>
>> +/* { dg-final { scan-assembler-times "maddld " 2 } } */
>> +/* { dg-final { scan-assembler-not "mulld " } } */
>> +/* { dg-final { scan-assembler-not "add " } } */
>
> You can easier write this using \m and \M, a bit more exact even:
>
> /* { dg-final { scan-assembler-times {\mmaddld\M} 2 } } *As the file name is madld-1.c, the resulting assembly file contains
.file "maddld-1.c"
This will cause the test case to fail.
I will replace it with the following statement
/* { dg-final { scan-assembler-times {\mmaddld\s} 2 } }
> /* { dg-final { scan-assembler-not {\mmul} } } */
> /* { dg-final { scan-assembler-not {\madd} } } */
>
> Which allows only the exact mnemonic "maddld", and disallows anything
> starting with "mul" or "add".
>
> Okay for trunk, with the testcase improvements please. Thanks!
>
>
> Segher
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-06-26 5:07 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-24 6:00 [PATCH] [RS6000] Change maddld match_operand from DI to GPR Li Jia He
2019-06-24 6:45 ` Kewen.Lin
2019-06-24 7:19 ` Segher Boessenkool
2019-06-24 7:43 ` Kewen.Lin
2019-06-24 7:49 ` Kewen.Lin
2019-06-24 8:16 ` Segher Boessenkool
2019-06-24 8:02 ` Segher Boessenkool
2019-06-24 8:10 ` Kewen.Lin
2019-06-24 7:39 ` Segher Boessenkool
2019-06-26 5:07 ` Li Jia He
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).