diff mbox series

[1/2,rs6000] int128 sign extention instructions (partial prereq)

Message ID a88319d1736ccdd1af6b19cd647215e8954a6865.camel@vnet.ibm.com
State New
Headers show
Series [1/2,rs6000] int128 sign extention instructions (partial prereq) | expand

Commit Message

will schmidt Sept. 24, 2020, 3:59 p.m. UTC 
[PATCH, rs6000] int128 sign extention instructions (partial prereq)
    
Hi
  This is a sub-set of the 128-bit sign extension support patch series
that I believe will be fully implemented in a subsequent patch from Carl.
This is a necessary pre-requisite for the vector-load/store rightmost
element patch that follows in this thread.

Thanks,
-Will
    
gcc/ChangeLog:
	* config/rs6000/rs6000.md (enum c_enum): Add UNSPEC_EXTENDDITI2
	and UNSPEC_MTVSRD_DITI_W1 entries.
	(mtvsrdd_diti_w1, extendditi2_vector): New define_insns.
	(extendditi2): New define_expand.

Comments

Segher Boessenkool Sept. 24, 2020, 10:39 p.m. UTC | #1 
Hi!

On Thu, Sep 24, 2020 at 10:59:09AM -0500, will schmidt wrote:
>   This is a sub-set of the 128-bit sign extension support patch series
> that I believe will be fully implemented in a subsequent patch from Carl.
> This is a necessary pre-requisite for the vector-load/store rightmost
> element patch that follows in this thread.

> 	* config/rs6000/rs6000.md (enum c_enum): Add UNSPEC_EXTENDDITI2
> 	and UNSPEC_MTVSRD_DITI_W1 entries.

(The define_c_enum is called "unspec", not "c_enum".)

These should really be coded not as unspecs, but as normal RTL code?
That way, it can be optimised.

> +;; Move DI value from GPR to TI mode in VSX register, word 1.
> +(define_insn "mtvsrdd_diti_w1"
> +  [(set (match_operand:TI 0 "register_operand" "=wa")
> +	(unspec:TI [(match_operand:DI 1 "register_operand" "r")]
> +		   UNSPEC_MTVSRD_DITI_W1))]
> +  "TARGET_POWERPC64 && TARGET_DIRECT_MOVE"
> +  "mtvsrdd %x0,0,%1"
> +  [(set_attr "type" "vecsimple")])

(Hrm, we should have had an extended mnemonic for this, "mtvsrld".  Oh
well.)

This should be in vsx.md?

And, please just extend vsx_concat for this?  Maybe using
reg_or_zero_operand?

> +;; Sign extend 64-bit value in TI reg, word 1, to 128-bit value in TI reg
> +(define_insn "extendditi2_vector"
> +  [(set (match_operand:TI 0 "gpc_reg_operand" "=v")
> +        (unspec:TI [(match_operand:TI 1 "gpc_reg_operand" "v")]
> +         UNSPEC_EXTENDDITI2))]
> +  "TARGET_POWER10"
> +  "vextsd2q %0,%1"
> +  [(set_attr "type" "exts")])

This should use something with sign_extend.

Okay for trunk.  Thanks!  But the unspecs really need to go sooner
rather than later (these are by far not the only ones, so :-( ).


Segher
Pat Haugen Sept. 25, 2020, 7:54 p.m. UTC | #2 
On 9/24/20 10:59 AM, will schmidt via Gcc-patches wrote:
> +;; Move DI value from GPR to TI mode in VSX register, word 1.
> +(define_insn "mtvsrdd_diti_w1"
> +  [(set (match_operand:TI 0 "register_operand" "=wa")
> +	(unspec:TI [(match_operand:DI 1 "register_operand" "r")]
> +		   UNSPEC_MTVSRD_DITI_W1))]
> +  "TARGET_POWERPC64 && TARGET_DIRECT_MOVE"
> +  "mtvsrdd %x0,0,%1"
> +  [(set_attr "type" "vecsimple")])

"vecmove" (since I just updated the other uses).

> +
> +;; Sign extend 64-bit value in TI reg, word 1, to 128-bit value in TI reg
> +(define_insn "extendditi2_vector"
> +  [(set (match_operand:TI 0 "gpc_reg_operand" "=v")
> +        (unspec:TI [(match_operand:TI 1 "gpc_reg_operand" "v")]
> +         UNSPEC_EXTENDDITI2))]
> +  "TARGET_POWER10"
> +  "vextsd2q %0,%1"
> +  [(set_attr "type" "exts")])

"vecexts".

> +
> +(define_expand "extendditi2"
> +  [(set (match_operand:TI 0 "gpc_reg_operand")
> +        (sign_extend:DI (match_operand:DI 1 "gpc_reg_operand")))]
> +  "TARGET_POWER10"
> +  {
> +    /* Move 64-bit src from GPR to vector reg and sign extend to 128-bits */
> +    rtx temp = gen_reg_rtx (TImode);
> +    emit_insn (gen_mtvsrdd_diti_w1 (temp, operands[1]));
> +    emit_insn (gen_extendditi2_vector (operands[0], temp));
> +    DONE;
> +  }
> +  [(set_attr "type" "exts")])

Don't need "type" attr on define_expand since the type will come from the 2 individual insns emitted.

Thanks,
Pat
Segher Boessenkool Sept. 25, 2020, 11:41 p.m. UTC | #3 
On Fri, Sep 25, 2020 at 02:54:01PM -0500, Pat Haugen wrote:
> > +(define_expand "extendditi2"
> > +  [(set (match_operand:TI 0 "gpc_reg_operand")
> > +        (sign_extend:DI (match_operand:DI 1 "gpc_reg_operand")))]
> > +  "TARGET_POWER10"
> > +  {
> > +    /* Move 64-bit src from GPR to vector reg and sign extend to 128-bits */
> > +    rtx temp = gen_reg_rtx (TImode);
> > +    emit_insn (gen_mtvsrdd_diti_w1 (temp, operands[1]));
> > +    emit_insn (gen_extendditi2_vector (operands[0], temp));
> > +    DONE;
> > +  }
> > +  [(set_attr "type" "exts")])
> 
> Don't need "type" attr on define_expand since the type will come from the 2 individual insns emitted.

Yeah good point, those attrs do not even do anything on the expand as
far as I can see (do *any* attrs, even?)


Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index 9c5a228..7d0b296 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -150,10 +150,12 @@ 
    UNSPEC_PLTSEQ
    UNSPEC_PLT16_HA
    UNSPEC_CFUGED
    UNSPEC_CNTLZDM
    UNSPEC_CNTTZDM
+   UNSPEC_EXTENDDITI2
+   UNSPEC_MTVSRD_DITI_W1
    UNSPEC_PDEPD
    UNSPEC_PEXTD
   ])
 
 ;;
@@ -963,10 +965,41 @@ 
   ""
   [(set_attr "type" "shift")
    (set_attr "dot" "yes")
    (set_attr "length" "4,8")])
 
+;; Move DI value from GPR to TI mode in VSX register, word 1.
+(define_insn "mtvsrdd_diti_w1"
+  [(set (match_operand:TI 0 "register_operand" "=wa")
+	(unspec:TI [(match_operand:DI 1 "register_operand" "r")]
+		   UNSPEC_MTVSRD_DITI_W1))]
+  "TARGET_POWERPC64 && TARGET_DIRECT_MOVE"
+  "mtvsrdd %x0,0,%1"
+  [(set_attr "type" "vecsimple")])
+
+;; Sign extend 64-bit value in TI reg, word 1, to 128-bit value in TI reg
+(define_insn "extendditi2_vector"
+  [(set (match_operand:TI 0 "gpc_reg_operand" "=v")
+        (unspec:TI [(match_operand:TI 1 "gpc_reg_operand" "v")]
+         UNSPEC_EXTENDDITI2))]
+  "TARGET_POWER10"
+  "vextsd2q %0,%1"
+  [(set_attr "type" "exts")])
+
+(define_expand "extendditi2"
+  [(set (match_operand:TI 0 "gpc_reg_operand")
+        (sign_extend:DI (match_operand:DI 1 "gpc_reg_operand")))]
+  "TARGET_POWER10"
+  {
+    /* Move 64-bit src from GPR to vector reg and sign extend to 128-bits */
+    rtx temp = gen_reg_rtx (TImode);
+    emit_insn (gen_mtvsrdd_diti_w1 (temp, operands[1]));
+    emit_insn (gen_extendditi2_vector (operands[0], temp));
+    DONE;
+  }
+  [(set_attr "type" "exts")])
+
 
 (define_insn "extendqi<mode>2"
   [(set (match_operand:EXTQI 0 "gpc_reg_operand" "=r,?*v")
 	(sign_extend:EXTQI (match_operand:QI 1 "gpc_reg_operand" "r,?*v")))]
   ""