| Message ID | adf6f3a2-0b43-4155-b564-729cc0a9be9d@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [rs6000] Use bcdsub. instead of bcdadd. for bcd invalid number checking | expand |
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 } } */ >
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
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 } } */