[PATCH] rs6000: Support more short/char to float conversion

Kewen.Lin linkw@linux.ibm.com
Thu Jun 10 09:32:23 GMT 2021


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.
-------------- next part --------------
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}   } } */


More information about the Gcc-patches mailing list