Re: [PATCH] rs6000: fmr gets used instead of faster xxlor [PR93571]

[Ajit Agarwal <aagarwa1@linux.ibm.com>]

Hello All:

Here is the patch that uses xxlor instead of fmr where possible.
Performance results shows that fmr is better in power9 and 
power10 architectures whereas xxlor is better in power7 and
power 8 architectures.

Bootstrapped and regtested powepc64-linux-gnu.

Thanks & Regards
Ajit

	rs6000: Use xxlor instead of fmr where possible

	This patch replaces fmr with xxlor instruction for power7
	and power8 architectures whereas for power9 and power10
	replaces xxlor with fmr instruction.

	Perf measurement results:

	Power9 fmr:  201,847,661 cycles.
	Power9 xxlor: 201,877,78 cycles.
	Power8 fmr: 201,057,795 cycles.
        Power8 xxlor: 201,004,671 cycles.

	2023-02-24  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>

gcc/ChangeLog:

	* config/rs6000/rs6000.md (*movdf_hardfloat64): Use xxlor
	for power7 and power8 and fmr for power9 and power10.
---
 gcc/config/rs6000/rs6000.md | 46 +++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 81bffb04ceb..1253b8622a7 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -354,7 +354,7 @@ (define_attr "cpu"
   (const (symbol_ref "(enum attr_cpu) rs6000_tune")))
 
 ;; The ISA we implement.
-(define_attr "isa" "any,p5,p6,p7,p7v,p8v,p9,p9v,p9kf,p9tf,p10"
+(define_attr "isa" "any,p5,p6,p7,p7v,p8v,p9,p9v,p9kf,p9tf,p7p8,p10"
   (const_string "any"))
 
 ;; Is this alternative enabled for the current CPU/ISA/etc.?
@@ -402,6 +402,11 @@ (define_attr "enabled" ""
      (and (eq_attr "isa" "p10")
 	  (match_test "TARGET_POWER10"))
      (const_int 1)
+      
+     (and (eq_attr "isa" "p7p8")
+	  (match_test "TARGET_VSX && !TARGET_P9_VECTOR"))
+     (const_int 1)
+
     ] (const_int 0)))
 
 ;; If this instruction is microcoded on the CELL processor
@@ -8436,27 +8441,29 @@ (define_insn "*mov<mode>_softfloat32"
 
 (define_insn "*mov<mode>_hardfloat64"
   [(set (match_operand:FMOVE64 0 "nonimmediate_operand"
-           "=m,           d,          d,          <f64_p9>,   wY,
-             <f64_av>,    Z,          <f64_vsx>,  <f64_vsx>,  !r,
-             YZ,          r,          !r,         *c*l,       !r,
-            *h,           r,          <f64_dm>,   wa")
+           "=m,           d,          <f64_vsx>,  <f64_p9>,   wY,
+            <f64_av>,     Z,          wa,         <f64_vsx>,  !r,
+            YZ,           r,          !r,         *c*l,       !r,
+            *h,           r,          <f64_dm>,   d,          wn,
+            wa")
 	(match_operand:FMOVE64 1 "input_operand"
-            "d,           m,          d,          wY,         <f64_p9>,
-             Z,           <f64_av>,   <f64_vsx>,  <zero_fp>,  <zero_fp>,
+            "d,           m,          <f64_vsx>,  wY,         <f64_p9>,
+             Z,           <f64_av>,   wa,         <zero_fp>,  <zero_fp>,
              r,           YZ,         r,          r,          *h,
-             0,           <f64_dm>,   r,          eP"))]
+             0,           <f64_dm>,   r,          d,          wn,
+             eP"))]
   "TARGET_POWERPC64 && TARGET_HARD_FLOAT
    && (gpc_reg_operand (operands[0], <MODE>mode)
        || gpc_reg_operand (operands[1], <MODE>mode))"
   "@
    stfd%U0%X0 %1,%0
    lfd%U1%X1 %0,%1
-   fmr %0,%1
+   xxlor %x0,%x1,%x1
    lxsd %0,%1
    stxsd %1,%0
    lxsdx %x0,%y1
    stxsdx %x1,%y0
-   xxlor %x0,%x1,%x1
+   fmr %0,%1
    xxlxor %x0,%x0,%x0
    li %0,0
    std%U0%X0 %1,%0
@@ -8467,23 +8474,28 @@ (define_insn "*mov<mode>_hardfloat64"
    nop
    mfvsrd %0,%x1
    mtvsrd %x0,%1
+   fmr %0,%1
+   fmr %0,%1
    #"
   [(set_attr "type"
-            "fpstore,     fpload,     fpsimple,   fpload,     fpstore,
+            "fpstore,     fpload,     veclogical, fpload,     fpstore,
              fpload,      fpstore,    veclogical, veclogical, integer,
              store,       load,       *,          mtjmpr,     mfjmpr,
-             *,           mfvsr,      mtvsr,      vecperm")
+             *,           mfvsr,      mtvsr,      fpsimple,   fpsimple,
+             vecperm")
    (set_attr "size" "64")
    (set_attr "isa"
-            "*,           *,          *,          p9v,        p9v,
-             p7v,         p7v,        *,          *,          *,
-             *,           *,          *,          *,          *,
-             *,           p8v,        p8v,        p10")
+            "*,           *,          p7p8,        p9v,        p9v,
+             p7v,         p7v,        *,           *,          *,
+             *,           *,          *,           *,          *,
+             *,           p8v,        p8v,         *,          *,
+             p10")
    (set_attr "prefixed"
             "*,           *,          *,          *,          *,
              *,           *,          *,          *,          *,
              *,           *,          *,          *,          *,
-             *,           *,          *,          *")])
+             *,           *,          *,          *,          *,
+             *")])
 
 ;;           STD      LD       MR      MT<SPR> MF<SPR> G-const
 ;;           H-const  F-const  Special
-- 
2.31.1



On 22/02/23 3:58 pm, Ajit Agarwal via Gcc-patches wrote:
> 
> 
> On 21/02/23 7:39 pm, Segher Boessenkool wrote:
>> On Tue, Feb 21, 2023 at 06:00:52PM +0530, Ajit Agarwal wrote:
>>> On 21/02/23 4:34 pm, Segher Boessenkool wrote:
>>>> Please domn't use a switch, it isn't needed.  Instead use the "isa"
>>>> attribute (with p7v here), and put the preferred alternative first.
>>>
>>> I am not sure how this is possible without switch and using only "isa".
>>
>> You have the "p7v" "xxlor" alternative earlier than the "*" "fmr"
>> alternative.  You can have an "xxlor" for contraints "d", but probably
>> the best (and certainly the easiest) is to just move the existing
>> xxlor to before fmr.
>>
>> Oh, the existing xxlor alternative is implicitly isa p7v, the "wa"
>> constraint causes that.  It may be nicer to mark it explicitly p7v as
>> well, nicer for the reader.
>>
> 
> If I do the above, for power9 it selects xxlor instead of fmr.
> 
>> Btw, please update the other similar patterns at the same time?  There
>> are eight patterns with fmr in rs6000.md (the four in dfp.md should
>> probably not be touched); not all are similar so should be in separate
>> patches, if changed at all, but a bunch are completely analogous so
>> should not diverge.
>>
>> (It is fine to first do this one pattern only, until we have worked out
>> all kinks, but all should be committed at the same time).
>>
>> Thanks,
>>
>>
>> Segher