* [PATCH, rs6000] Use bcdsub. instead of bcdadd. for bcd invalid number checking
@ 2024-04-18 2:01 HAO CHEN GUI
2024-04-18 3:14 ` Kewen.Lin
0 siblings, 1 reply; 3+ messages in thread
From: HAO CHEN GUI @ 2024-04-18 2:01 UTC (permalink / raw)
To: gcc-patches; +Cc: Segher Boessenkool, David, Kewen.Lin, Peter Bergner
Hi,
This patch replace bcdadd. with bcdsub. for bcd invalid number checking.
bcdadd on two same numbers might cause overflow which also set
overflow/invalid bit so that we can't distinguish it's invalid or overflow.
The bcdsub doesn't have the problem as subtracting on two same number never
causes overflow.
Bootstrapped and tested on powerpc64-linux BE and LE with no
regressions. Is it OK for the trunk?
Thanks
Gui Haochen
ChangeLog
rs6000: Use bcdsub. instead of bcdadd. for bcd invalid number checking
bcdadd. might causes overflow which also set the overflow/invalid bit.
bcdsub. doesn't have the issue when do subtracting on two same bcd number.
gcc/
* config/rs6000/altivec.md (*bcdinvalid_<mode>): Replace bcdadd
with bcdsub.
(bcdinvalid_<mode>): Likewise.
gcc/testsuite/
* gcc.target/powerpc/bcd-4.c: Adjust the number of bcdadd and
bcdsub.
patch.diff
diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 4d4c94ff0a0..bb20441c096 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -4586,18 +4586,18 @@ (define_insn "*bcdinvalid_<mode>"
[(set (reg:CCFP CR6_REGNO)
(compare:CCFP
(unspec:V2DF [(match_operand:VBCD 1 "register_operand" "v")]
- UNSPEC_BCDADD)
+ UNSPEC_BCDSUB)
(match_operand:V2DF 2 "zero_constant" "j")))
(clobber (match_scratch:VBCD 0 "=v"))]
"TARGET_P8_VECTOR"
- "bcdadd. %0,%1,%1,0"
+ "bcdsub. %0,%1,%1,0"
[(set_attr "type" "vecsimple")])
(define_expand "bcdinvalid_<mode>"
[(parallel [(set (reg:CCFP CR6_REGNO)
(compare:CCFP
(unspec:V2DF [(match_operand:VBCD 1 "register_operand")]
- UNSPEC_BCDADD)
+ UNSPEC_BCDSUB)
(match_dup 2)))
(clobber (match_scratch:VBCD 3))])
(set (match_operand:SI 0 "register_operand")
diff --git a/gcc/testsuite/gcc.target/powerpc/bcd-4.c b/gcc/testsuite/gcc.target/powerpc/bcd-4.c
index 2c7041c4d32..6d2c59ef792 100644
--- a/gcc/testsuite/gcc.target/powerpc/bcd-4.c
+++ b/gcc/testsuite/gcc.target/powerpc/bcd-4.c
@@ -2,8 +2,8 @@
/* { dg-require-effective-target int128 } */
/* { dg-require-effective-target p9vector_hw } */
/* { dg-options "-mdejagnu-cpu=power9 -O2 -save-temps" } */
-/* { dg-final { scan-assembler-times {\mbcdadd\M} 7 } } */
-/* { dg-final { scan-assembler-times {\mbcdsub\M} 18 } } */
+/* { dg-final { scan-assembler-times {\mbcdadd\M} 5 } } */
+/* { dg-final { scan-assembler-times {\mbcdsub\M} 20 } } */
/* { dg-final { scan-assembler-times {\mbcds\M} 2 } } */
/* { dg-final { scan-assembler-times {\mdenbcdq\M} 1 } } */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH, rs6000] Use bcdsub. instead of bcdadd. for bcd invalid number checking
2024-04-18 2:01 [PATCH, rs6000] Use bcdsub. instead of bcdadd. for bcd invalid number checking HAO CHEN GUI
@ 2024-04-18 3:14 ` Kewen.Lin
2024-04-18 10:35 ` Segher Boessenkool
0 siblings, 1 reply; 3+ messages in thread
From: Kewen.Lin @ 2024-04-18 3:14 UTC (permalink / raw)
To: HAO CHEN GUI; +Cc: Segher Boessenkool, David, Peter Bergner, gcc-patches
Hi,
on 2024/4/18 10:01, HAO CHEN GUI wrote:
> Hi,
> This patch replace bcdadd. with bcdsub. for bcd invalid number checking.
> bcdadd on two same numbers might cause overflow which also set
> overflow/invalid bit so that we can't distinguish it's invalid or overflow.
> The bcdsub doesn't have the problem as subtracting on two same number never
> causes overflow.
>
> Bootstrapped and tested on powerpc64-linux BE and LE with no
> regressions. Is it OK for the trunk?
Considering that this issue affects some basic functionality of bcd bifs
and the fix itself is simple and very safe, OK for trunk, thanks for fixing!
BR,
Kewen
>
> Thanks
> Gui Haochen
>
> ChangeLog
> rs6000: Use bcdsub. instead of bcdadd. for bcd invalid number checking
>
> bcdadd. might causes overflow which also set the overflow/invalid bit.
> bcdsub. doesn't have the issue when do subtracting on two same bcd number.
>
> gcc/
> * config/rs6000/altivec.md (*bcdinvalid_<mode>): Replace bcdadd
> with bcdsub.
> (bcdinvalid_<mode>): Likewise.
>
> gcc/testsuite/
> * gcc.target/powerpc/bcd-4.c: Adjust the number of bcdadd and
> bcdsub.
>
> patch.diff
> diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
> index 4d4c94ff0a0..bb20441c096 100644
> --- a/gcc/config/rs6000/altivec.md
> +++ b/gcc/config/rs6000/altivec.md
> @@ -4586,18 +4586,18 @@ (define_insn "*bcdinvalid_<mode>"
> [(set (reg:CCFP CR6_REGNO)
> (compare:CCFP
> (unspec:V2DF [(match_operand:VBCD 1 "register_operand" "v")]
> - UNSPEC_BCDADD)
> + UNSPEC_BCDSUB)
> (match_operand:V2DF 2 "zero_constant" "j")))
> (clobber (match_scratch:VBCD 0 "=v"))]
> "TARGET_P8_VECTOR"
> - "bcdadd. %0,%1,%1,0"
> + "bcdsub. %0,%1,%1,0"
> [(set_attr "type" "vecsimple")])
>
> (define_expand "bcdinvalid_<mode>"
> [(parallel [(set (reg:CCFP CR6_REGNO)
> (compare:CCFP
> (unspec:V2DF [(match_operand:VBCD 1 "register_operand")]
> - UNSPEC_BCDADD)
> + UNSPEC_BCDSUB)
> (match_dup 2)))
> (clobber (match_scratch:VBCD 3))])
> (set (match_operand:SI 0 "register_operand")
> diff --git a/gcc/testsuite/gcc.target/powerpc/bcd-4.c b/gcc/testsuite/gcc.target/powerpc/bcd-4.c
> index 2c7041c4d32..6d2c59ef792 100644
> --- a/gcc/testsuite/gcc.target/powerpc/bcd-4.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bcd-4.c
> @@ -2,8 +2,8 @@
> /* { dg-require-effective-target int128 } */
> /* { dg-require-effective-target p9vector_hw } */
> /* { dg-options "-mdejagnu-cpu=power9 -O2 -save-temps" } */
> -/* { dg-final { scan-assembler-times {\mbcdadd\M} 7 } } */
> -/* { dg-final { scan-assembler-times {\mbcdsub\M} 18 } } */
> +/* { dg-final { scan-assembler-times {\mbcdadd\M} 5 } } */
> +/* { dg-final { scan-assembler-times {\mbcdsub\M} 20 } } */
> /* { dg-final { scan-assembler-times {\mbcds\M} 2 } } */
> /* { dg-final { scan-assembler-times {\mdenbcdq\M} 1 } } */
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH, rs6000] Use bcdsub. instead of bcdadd. for bcd invalid number checking
2024-04-18 3:14 ` Kewen.Lin
@ 2024-04-18 10:35 ` Segher Boessenkool
0 siblings, 0 replies; 3+ messages in thread
From: Segher Boessenkool @ 2024-04-18 10:35 UTC (permalink / raw)
To: Kewen.Lin; +Cc: HAO CHEN GUI, David, Peter Bergner, gcc-patches
On Thu, Apr 18, 2024 at 11:14:42AM +0800, Kewen.Lin wrote:
> on 2024/4/18 10:01, HAO CHEN GUI wrote:
> > This patch replace bcdadd. with bcdsub. for bcd invalid number checking.
> > bcdadd on two same numbers might cause overflow which also set
> > overflow/invalid bit so that we can't distinguish it's invalid or overflow.
> > The bcdsub doesn't have the problem as subtracting on two same number never
> > causes overflow.
> >
> > Bootstrapped and tested on powerpc64-linux BE and LE with no
> > regressions. Is it OK for the trunk?
>
> Considering that this issue affects some basic functionality of bcd bifs
> and the fix itself is simple and very safe, OK for trunk, thanks for fixing!
Yup. If a number X is invalid the X-X calculation might set the
overflow flag as well, but we cannot see that difference at all anyway,
it always has set the invalid flag after all.
Thanks!
Segher
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-04-18 10:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-18 2:01 [PATCH, rs6000] Use bcdsub. instead of bcdadd. for bcd invalid number checking HAO CHEN GUI
2024-04-18 3:14 ` Kewen.Lin
2024-04-18 10:35 ` Segher Boessenkool
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).