* [PATCH] rs6000: Support more short/char to float conversion @ 2021-05-07 2:30 Kewen.Lin 2021-05-26 3:02 ` PING^1 " Kewen.Lin 2021-06-09 23:23 ` Segher Boessenkool 0 siblings, 2 replies; 8+ messages in thread From: Kewen.Lin @ 2021-05-07 2:30 UTC (permalink / raw) To: GCC Patches; +Cc: Bill Schmidt, Segher Boessenkool, David Edelsohn [-- Attachment #1: Type: text/plain, Size: 699 bytes --] Hi, For some cases that when we load unsigned char/short values from the appropriate unsigned char/short memories and convert them to double/single precision floating point value, there would be implicit conversions to int first. It makes GCC not leverage the P9 instructions lxsibzx/lxsihzx. This patch is to add the related define_insn_and_split to support this kind of scenario. Bootstrapped/regtested on powerpc64le-linux-gnu P9 and powerpc64-linux-gnu P8. Is it ok for trunk? BR, Kewen ------ gcc/ChangeLog: * config/rs6000/rs6000.md (floatsi<SFDF:mode>2_lfiwax_<QHI:mode>_mem_zext): New define_insn_and_split. gcc/testsuite/ChangeLog: * gcc.target/powerpc/p9-fpcvt-3.c: New test. [-- Attachment #2: 0003-rs6000-Support-more-short-char-to-float-conversion.patch --] [-- Type: text/plain, Size: 3074 bytes --] --- gcc/config/rs6000/rs6000.md | 22 +++++++++++++++ gcc/testsuite/gcc.target/powerpc/p9-fpcvt-3.c | 27 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 gcc/testsuite/gcc.target/powerpc/p9-fpcvt-3.c diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index c8cdc42533c..3ac7ed20852 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -5504,6 +5504,28 @@ (define_insn_and_split "floatsi<mode>2_lfiwax_mem" [(set_attr "length" "8") (set_attr "type" "fpload")]) +(define_insn_and_split "floatsi<SFDF:mode>2_lfiwax_<QHI:mode>_mem_zext" + [(set (match_operand:SFDF 0 "gpc_reg_operand" "=d,<Fv>") + (float:SFDF + (zero_extend:SI + (match_operand:QHI 1 "indexed_or_indirect_operand" "Z,Z")))) + (clobber (match_scratch:DI 2 "=d,wa"))] + "TARGET_HARD_FLOAT && <SI_CONVERT_FP> && TARGET_P9_VECTOR + && TARGET_POWERPC64 && TARGET_DIRECT_MOVE" + "#" + "&& 1" + [(pc)] +{ + operands[1] = rs6000_force_indexed_or_indirect_mem (operands[1]); + if (GET_CODE (operands[2]) == SCRATCH) + operands[2] = gen_reg_rtx (DImode); + emit_insn (gen_zero_extendhidi2 (operands[2], operands[1])); + emit_insn (gen_floatdi<SFDF:mode>2 (operands[0], operands[2])); + DONE; +} + [(set_attr "length" "8") + (set_attr "type" "fpload")]) + (define_insn "lfiwzx" [(set (match_operand:DI 0 "gpc_reg_operand" "=d,wa,wa,wa") (unspec:DI [(match_operand:SI 1 "reg_or_indexed_operand" "Z,Z,r,wa")] diff --git a/gcc/testsuite/gcc.target/powerpc/p9-fpcvt-3.c b/gcc/testsuite/gcc.target/powerpc/p9-fpcvt-3.c new file mode 100644 index 00000000000..d3bbe36b759 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/p9-fpcvt-3.c @@ -0,0 +1,27 @@ +/* { dg-do compile { target lp64 } } */ +/* { dg-require-effective-target powerpc_p9vector_ok } */ +/* { dg-options "-mdejagnu-cpu=power9 -O2" } */ + +/* Note that for unsigned cases, the differences from those ones in + p9-fpcvt-2.c is that they will be converted to int implicitly first + and then to floating point. */ + +double sc_df (signed char *p, double df) { return *p + df; } +double uc_df (unsigned char *p, double df) { return *p + df; } +double ss_df (signed short *p, double df) { return *p + df; } +double us_df (unsigned short *p, double df) { return *p + df; } + +float sc_sf (signed char *p, float sf) { return *p + sf; } +float uc_sf (unsigned char *p, float sf) { return *p + sf; } +float ss_sf (signed short *p, float sf) { return *p + sf; } +float us_sf (unsigned short *p, float sf) { return *p + sf; } + +/* { dg-final { scan-assembler "lxsibzx" } } */ +/* { dg-final { scan-assembler "lxsihzx" } } */ +/* { dg-final { scan-assembler "vextsb2d" } } */ +/* { dg-final { scan-assembler "vextsh2d" } } */ +/* { dg-final { scan-assembler-not "mfvsrd" } } */ +/* { dg-final { scan-assembler-not "mfvsrwz" } } */ +/* { dg-final { scan-assembler-not "mtvsrd" } } */ +/* { dg-final { scan-assembler-not "mtvsrwa" } } */ +/* { dg-final { scan-assembler-not "mtvsrwz" } } */ -- 2.17.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* PING^1 [PATCH] rs6000: Support more short/char to float conversion 2021-05-07 2:30 [PATCH] rs6000: Support more short/char to float conversion Kewen.Lin @ 2021-05-26 3:02 ` Kewen.Lin 2021-06-09 2:29 ` PING^2 " Kewen.Lin 2021-06-09 23:23 ` Segher Boessenkool 1 sibling, 1 reply; 8+ messages in thread From: Kewen.Lin @ 2021-05-26 3:02 UTC (permalink / raw) To: GCC Patches; +Cc: Bill Schmidt, David Edelsohn, Segher Boessenkool Hi, Gentle ping this: https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569792.html BR, Kewen on 2021/5/7 上午10:30, Kewen.Lin via Gcc-patches wrote: > Hi, > > For some cases that when we load unsigned char/short values from > the appropriate unsigned char/short memories and convert them to > double/single precision floating point value, there would be > implicit conversions to int first. It makes GCC not leverage the > P9 instructions lxsibzx/lxsihzx. This patch is to add the related > define_insn_and_split to support this kind of scenario. > > Bootstrapped/regtested on powerpc64le-linux-gnu P9 and > powerpc64-linux-gnu P8. > > Is it ok for trunk? > > BR, > Kewen > ------ > gcc/ChangeLog: > > * config/rs6000/rs6000.md > (floatsi<SFDF:mode>2_lfiwax_<QHI:mode>_mem_zext): New > define_insn_and_split. > > gcc/testsuite/ChangeLog: > > * gcc.target/powerpc/p9-fpcvt-3.c: New test. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* PING^2 [PATCH] rs6000: Support more short/char to float conversion 2021-05-26 3:02 ` PING^1 " Kewen.Lin @ 2021-06-09 2:29 ` Kewen.Lin 0 siblings, 0 replies; 8+ messages in thread From: Kewen.Lin @ 2021-06-09 2:29 UTC (permalink / raw) To: GCC Patches; +Cc: Bill Schmidt, Segher Boessenkool, David Edelsohn Hi, Gentle ping this: https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569792.html BR, Kewen on 2021/5/26 上午11:02, Kewen.Lin via Gcc-patches wrote: > Hi, > > Gentle ping this: > > https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569792.html > > > BR, > Kewen > > on 2021/5/7 上午10:30, Kewen.Lin via Gcc-patches wrote: >> Hi, >> >> For some cases that when we load unsigned char/short values from >> the appropriate unsigned char/short memories and convert them to >> double/single precision floating point value, there would be >> implicit conversions to int first. It makes GCC not leverage the >> P9 instructions lxsibzx/lxsihzx. This patch is to add the related >> define_insn_and_split to support this kind of scenario. >> >> Bootstrapped/regtested on powerpc64le-linux-gnu P9 and >> powerpc64-linux-gnu P8. >> >> Is it ok for trunk? >> >> BR, >> Kewen >> ------ >> gcc/ChangeLog: >> >> * config/rs6000/rs6000.md >> (floatsi<SFDF:mode>2_lfiwax_<QHI:mode>_mem_zext): New >> define_insn_and_split. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/powerpc/p9-fpcvt-3.c: New test. >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rs6000: Support more short/char to float conversion 2021-05-07 2:30 [PATCH] rs6000: Support more short/char to float conversion Kewen.Lin 2021-05-26 3:02 ` PING^1 " Kewen.Lin @ 2021-06-09 23:23 ` Segher Boessenkool 2021-06-10 9:32 ` Kewen.Lin 1 sibling, 1 reply; 8+ messages in thread From: Segher Boessenkool @ 2021-06-09 23:23 UTC (permalink / raw) To: Kewen.Lin; +Cc: GCC Patches, Bill Schmidt, David Edelsohn Hi! On Fri, May 07, 2021 at 10:30:38AM +0800, Kewen.Lin wrote: > For some cases that when we load unsigned char/short values from > the appropriate unsigned char/short memories and convert them to > double/single precision floating point value, there would be > implicit conversions to int first. It makes GCC not leverage the > P9 instructions lxsibzx/lxsihzx. This patch is to add the related > define_insn_and_split to support this kind of scenario. > +/* { dg-final { scan-assembler "lxsibzx" } } */ > +/* { dg-final { scan-assembler "lxsihzx" } } */ > +/* { dg-final { scan-assembler "vextsb2d" } } */ > +/* { dg-final { scan-assembler "vextsh2d" } } */ On my unpatched compiler all these already work, but you say they don't? For the first two I get lxsibzx 33,0,3 vextsb2d 0,1 xscvsxddp 0,32 fadd 1,0,1 blr and lbz 9,0(3) mtvsrwa 0,9 fcfid 0,0 fadd 1,0,1 blr is that different for you? In either case, use \m and \M please. > +/* { dg-final { scan-assembler-not "mfvsrd" } } */ > +/* { dg-final { scan-assembler-not "mfvsrwz" } } */ > +/* { dg-final { scan-assembler-not "mtvsrd" } } */ > +/* { dg-final { scan-assembler-not "mtvsrwa" } } */ > +/* { dg-final { scan-assembler-not "mtvsrwz" } } */ Here as well, or you could just do /* { dg-final { scan-assembler-not "\mm[tf]vsr" } } */ in this case, since no VSR<->GPR moves should happen at all. Segher ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rs6000: Support more short/char to float conversion 2021-06-09 23:23 ` Segher Boessenkool @ 2021-06-10 9:32 ` Kewen.Lin 2021-06-10 10:58 ` Segher Boessenkool 0 siblings, 1 reply; 8+ messages in thread From: Kewen.Lin @ 2021-06-10 9:32 UTC (permalink / raw) To: Segher Boessenkool; +Cc: GCC Patches, Bill Schmidt, David Edelsohn [-- Attachment #1: Type: text/plain, Size: 2346 bytes --] Hi Segher, Thanks for the review! on 2021/6/10 上午7:23, Segher Boessenkool wrote: > Hi! > > On Fri, May 07, 2021 at 10:30:38AM +0800, Kewen.Lin wrote: >> For some cases that when we load unsigned char/short values from >> the appropriate unsigned char/short memories and convert them to >> double/single precision floating point value, there would be >> implicit conversions to int first. It makes GCC not leverage the >> P9 instructions lxsibzx/lxsihzx. This patch is to add the related >> define_insn_and_split to support this kind of scenario. > >> +/* { dg-final { scan-assembler "lxsibzx" } } */ >> +/* { dg-final { scan-assembler "lxsihzx" } } */ >> +/* { dg-final { scan-assembler "vextsb2d" } } */ >> +/* { dg-final { scan-assembler "vextsh2d" } } */ > > On my unpatched compiler all these already work, but you say they don't? > Sorry for the confusion, the patch is to handle the unsigned char and short but the test case also has the test coverage on signed char and short, which follows the existing cases p9-fpcvt-{1,2}.c. As you stated, the signed part of cases are fine. > For the first two I get > lxsibzx 33,0,3 > vextsb2d 0,1 > xscvsxddp 0,32 > fadd 1,0,1 > blr > and > lbz 9,0(3) > mtvsrwa 0,9 > fcfid 0,0 > fadd 1,0,1 > blr > is that different for you? > I got the same output without the patch, applying the patch the second one becomes into: lxsibzx 0,0,3 fcfid 0,0 fadd 1,0,1 blr > In either case, use \m and \M please. > Fixed. >> +/* { dg-final { scan-assembler-not "mfvsrd" } } */ >> +/* { dg-final { scan-assembler-not "mfvsrwz" } } */ >> +/* { dg-final { scan-assembler-not "mtvsrd" } } */ >> +/* { dg-final { scan-assembler-not "mtvsrwa" } } */ >> +/* { dg-final { scan-assembler-not "mtvsrwz" } } */ > > Here as well, or you could just do > > /* { dg-final { scan-assembler-not "\mm[tf]vsr" } } */ > > in this case, since no VSR<->GPR moves should happen at all. > Thanks, updated accordingly. The patch v2 is attached, does it look better? BR, Kewen ------ gcc/ChangeLog: * config/rs6000/rs6000.md (floatsi<SFDF:mode>2_lfiwax_<QHI:mode>_mem_zext): New define_insn_and_split. gcc/testsuite/ChangeLog: * gcc.target/powerpc/p9-fpcvt-3.c: New test. [-- Attachment #2: conv_v2.diff --] [-- Type: text/plain, Size: 2551 bytes --] diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 3f59b544f6a..0574e10f923 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -5524,6 +5524,27 @@ (define_insn_and_split "floatsi<mode>2_lfiwax_mem" [(set_attr "length" "8") (set_attr "type" "fpload")]) +(define_insn_and_split "floatsi<SFDF:mode>2_lfiwax_<QHI:mode>_mem_zext" + [(set (match_operand:SFDF 0 "gpc_reg_operand" "=d,<Fv>") + (float:SFDF + (zero_extend:SI + (match_operand:QHI 1 "indexed_or_indirect_operand" "Z,Z")))) + (clobber (match_scratch:DI 2 "=d,wa"))] + "TARGET_HARD_FLOAT && <SI_CONVERT_FP> && TARGET_P9_VECTOR + && TARGET_POWERPC64 && TARGET_DIRECT_MOVE" + "#" + "&& 1" + [(pc)] +{ + if (GET_CODE (operands[2]) == SCRATCH) + operands[2] = gen_reg_rtx (DImode); + emit_insn (gen_zero_extendhidi2 (operands[2], operands[1])); + emit_insn (gen_floatdi<SFDF:mode>2 (operands[0], operands[2])); + DONE; +} + [(set_attr "length" "8") + (set_attr "type" "fpload")]) + (define_insn "lfiwzx" [(set (match_operand:DI 0 "gpc_reg_operand" "=d,wa,wa,wa") (unspec:DI [(match_operand:SI 1 "reg_or_indexed_operand" "Z,Z,r,wa")] diff --git a/gcc/testsuite/gcc.target/powerpc/p9-fpcvt-3.c b/gcc/testsuite/gcc.target/powerpc/p9-fpcvt-3.c new file mode 100644 index 00000000000..19701c84add --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/p9-fpcvt-3.c @@ -0,0 +1,23 @@ +/* { dg-do compile { target lp64 } } */ +/* { dg-require-effective-target powerpc_p9vector_ok } */ +/* { dg-options "-mdejagnu-cpu=power9 -O2" } */ + +/* Note that for unsigned cases, the differences from those ones in + p9-fpcvt-2.c is that they will be converted to int implicitly first + and then to floating point. */ + +double sc_df (signed char *p, double df) { return *p + df; } +double uc_df (unsigned char *p, double df) { return *p + df; } +double ss_df (signed short *p, double df) { return *p + df; } +double us_df (unsigned short *p, double df) { return *p + df; } + +float sc_sf (signed char *p, float sf) { return *p + sf; } +float uc_sf (unsigned char *p, float sf) { return *p + sf; } +float ss_sf (signed short *p, float sf) { return *p + sf; } +float us_sf (unsigned short *p, float sf) { return *p + sf; } + +/* { dg-final { scan-assembler {\mlxsibzx\M} } } */ +/* { dg-final { scan-assembler {\mlxsihzx\M} } } */ +/* { dg-final { scan-assembler {\mvextsb2d\M} } } */ +/* { dg-final { scan-assembler {\mvextsh2d\M} } } */ +/* { dg-final { scan-assembler-not {\mm[tf]vsr} } } */ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rs6000: Support more short/char to float conversion 2021-06-10 9:32 ` Kewen.Lin @ 2021-06-10 10:58 ` Segher Boessenkool 2021-06-11 12:45 ` Kewen.Lin 0 siblings, 1 reply; 8+ messages in thread From: Segher Boessenkool @ 2021-06-10 10:58 UTC (permalink / raw) To: Kewen.Lin; +Cc: GCC Patches, Bill Schmidt, David Edelsohn Hi! On Thu, Jun 10, 2021 at 05:32:23PM +0800, Kewen.Lin wrote: > +/* { dg-do compile { target lp64 } } */ One final thing: what requires lp64 here? Could you try without please? Okay for trunk with that considered. Thanks! Segher ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rs6000: Support more short/char to float conversion 2021-06-10 10:58 ` Segher Boessenkool @ 2021-06-11 12:45 ` Kewen.Lin 2021-06-12 14:21 ` Segher Boessenkool 0 siblings, 1 reply; 8+ messages in thread From: Kewen.Lin @ 2021-06-11 12:45 UTC (permalink / raw) To: Segher Boessenkool; +Cc: GCC Patches, Bill Schmidt, David Edelsohn Hi Segher, on 2021/6/10 下午6:58, Segher Boessenkool wrote: > Hi! > > On Thu, Jun 10, 2021 at 05:32:23PM +0800, Kewen.Lin wrote: >> +/* { dg-do compile { target lp64 } } */ > > One final thing: what requires lp64 here? Could you try without please? > The lp64 is required for lxsi[bh]zx and mvexts[bh]2d, without it -m32 testing will have some failures as below: FAIL: gcc.target/powerpc/p9-fpcvt-3.c scan-assembler \\mlxsibzx\\M FAIL: gcc.target/powerpc/p9-fpcvt-3.c scan-assembler \\mlxsihzx\\M FAIL: gcc.target/powerpc/p9-fpcvt-3.c scan-assembler \\\\M FAIL: gcc.target/powerpc/p9-fpcvt-3.c scan-assembler \\mvextsh2d\\M > Okay for trunk with that considered. Thanks! > Thanks! It's committed in r12-1378. BR, Kewen ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rs6000: Support more short/char to float conversion 2021-06-11 12:45 ` Kewen.Lin @ 2021-06-12 14:21 ` Segher Boessenkool 0 siblings, 0 replies; 8+ messages in thread From: Segher Boessenkool @ 2021-06-12 14:21 UTC (permalink / raw) To: Kewen.Lin; +Cc: GCC Patches, Bill Schmidt, David Edelsohn On Fri, Jun 11, 2021 at 08:45:53PM +0800, Kewen.Lin wrote: > on 2021/6/10 下午6:58, Segher Boessenkool wrote: > > On Thu, Jun 10, 2021 at 05:32:23PM +0800, Kewen.Lin wrote: > >> +/* { dg-do compile { target lp64 } } */ > > > > One final thing: what requires lp64 here? Could you try without please? > > The lp64 is required for lxsi[bh]zx and mvexts[bh]2d, without it > -m32 testing will have some failures as below: Ah, those extends of course (and the loads fall out from that). I should have seen that, thanks :-) Segher ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-06-12 14:22 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-07 2:30 [PATCH] rs6000: Support more short/char to float conversion Kewen.Lin 2021-05-26 3:02 ` PING^1 " Kewen.Lin 2021-06-09 2:29 ` PING^2 " Kewen.Lin 2021-06-09 23:23 ` Segher Boessenkool 2021-06-10 9:32 ` Kewen.Lin 2021-06-10 10:58 ` Segher Boessenkool 2021-06-11 12:45 ` Kewen.Lin 2021-06-12 14:21 ` 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).