public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, rs6000] Update instruction attributes for Power10
@ 2020-11-04 20:42 Pat Haugen
  2020-11-05 22:32 ` will schmidt
  2020-11-09 21:54 ` Segher Boessenkool
  0 siblings, 2 replies; 6+ messages in thread
From: Pat Haugen @ 2020-11-04 20:42 UTC (permalink / raw)
  To: GCC Patches
  Cc: Segher Boessenkool, David Edelsohn, Peter Bergner, Bill Schmidt

[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]

Update instruction attributes for Power10.


This patch updates the type/prefixed/dot/size attributes for various new instructions (and a couple existing that were incorrect) in preparation for the Power10 scheduling patch that will be following.

Bootstrap/regtest on powerpc64le (Power8/Power10) with no new regressions. Ok for trunk?

-Pat


2020-11-04  Pat Haugen  <pthaugen@linux.ibm.com>

gcc/
	* config/rs6000/altivec.md (vs<SLDB_lr>db_<mode>, xxspltiw_v4si,
	xxspltiw_v4sf_inst, xxspltidp_v2df_inst, xxsplti32dx_v4si_inst,
	xxsplti32dx_v4sf_inst, xxblend_<mode>, xxpermx_inst,
	vstrir_code_<mode>, vstrir_p_code_<mode>, vstril_code_<mode>,
	vstril_p_code_<mode>, altivec_lvsl_reg, altivec_lvsl_direct,
	altivec_lvsr_reg, altivec_lvsr_direct, xxeval, vcfuged, vclzdm,
	vctzdm, vpdepd, vpextd, vgnb, vclrlb, vclrrb): Update instruction
	attributes for Power10.
	* config/rs6000/dfp.md (extendddtd2, trunctddd2, *cmp<mode>_internal1,
	floatditd2, ftrunc<mode>2, fix<mode>di2, dfp_ddedpd_<mode>,
	dfp_denbcd_<mode>, dfp_dxex_<mode>, dfp_diex_<mode>,
	*dfp_sgnfcnc_<mode>, dfp_dscli_<mode>, dfp_dscri_<mode>): Likewise.
	* config/rs6000/mma.md (*movpoi, mma_<vvi4i4i8>, mma_<avvi4i4i8>,
	mma_<vvi4i4i2>, mma_<avvi4i4i2>, mma_<vvi4i4>, mma_<avvi4i4>,
	mma_<pvi4i2>, mma_<apvi4i2>, mma_<vvi4i4i4>, mma_<avvi4i4i4>):
	Likewise.
	* config/rs6000/rs6000.c (rs6000_final_prescan_insn): Only add 'p' for
	PREFIXED_YES.
	* config/rs6000/rs6000.md (define_attr "size"): Add 256.
	(define_attr "prefixed"): Add 'always'.
	(define_mode_attr bits): Add DD/TD modes.
	(cfuged, cntlzdm, cnttzdm, pdepd, pextd, bswaphi2_reg, bswapsi2_reg,
	bswapdi2_brd, setbc_<un>signed_<GPR:mode>,
	*setbcr_<un>signed_<GPR:mode>, *setnbc_<un>signed_<GPR:mode>,
	*setnbcr_<un>signed_<GPR:mode>): Update instruction attributes for
	Power10.
	* config/rs6000/sync.md (load_quadpti, store_quadpti, load_lockedpti,
	store_conditionalpti): Update instruction attributes for Power10.
	* config/rs6000/vsx.md (*xvtlsbb_internal, xxgenpcvm_<mode>_internal,
	vextractl<mode>_internal, vextractr<mode>_internal,
	vinsertvl_internal_<mode>, vinsertvr_internal_<mode>,
	vinsertgl_internal_<mode>, vinsertgr_internal_<mode>,
	vreplace_elt_<mode>_inst): Likewise.


[-- Attachment #2: p10_insn-attr.diff --]
[-- Type: text/plain, Size: 24450 bytes --]

diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 0a2e634d6b0..76191ba4107 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -819,7 +819,7 @@ (define_insn "vs<SLDB_lr>db_<mode>"
 	      VSHIFT_DBL_LR))]
   "TARGET_POWER10"
   "vs<SLDB_lr>dbi %0,%1,%2,%3"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")])
 
 (define_insn "xxspltiw_v4si"
   [(set (match_operand:V4SI 0 "register_operand" "=wa")
@@ -827,7 +827,8 @@ (define_insn "xxspltiw_v4si"
 		     UNSPEC_XXSPLTIW))]
  "TARGET_POWER10"
  "xxspltiw %x0,%1"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecperm")
+  (set_attr "prefixed" "always")])
 
 (define_expand "xxspltiw_v4sf"
   [(set (match_operand:V4SF 0 "register_operand" "=wa")
@@ -846,7 +847,8 @@ (define_insn "xxspltiw_v4sf_inst"
 		     UNSPEC_XXSPLTIW))]
  "TARGET_POWER10"
  "xxspltiw %x0,%1"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecperm")
+  (set_attr "prefixed" "always")])
 
 (define_expand "xxspltidp_v2df"
   [(set (match_operand:V2DF 0 "register_operand" )
@@ -865,7 +867,8 @@ (define_insn "xxspltidp_v2df_inst"
 		     UNSPEC_XXSPLTID))]
   "TARGET_POWER10"
   "xxspltidp %x0,%1"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")
+   (set_attr "prefixed" "always")])
 
 (define_expand "xxsplti32dx_v4si"
   [(set (match_operand:V4SI 0 "register_operand" "=wa")
@@ -894,7 +897,8 @@ (define_insn "xxsplti32dx_v4si_inst"
 		     UNSPEC_XXSPLTI32DX))]
   "TARGET_POWER10"
   "xxsplti32dx %x0,%2,%3"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")
+   (set_attr "prefixed" "always")])
 
 (define_expand "xxsplti32dx_v4sf"
   [(set (match_operand:V4SF 0 "register_operand" "=wa")
@@ -922,7 +926,8 @@ (define_insn "xxsplti32dx_v4sf_inst"
 		     UNSPEC_XXSPLTI32DX))]
   "TARGET_POWER10"
   "xxsplti32dx %x0,%2,%3"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")
+   (set_attr "prefixed" "always")])
 
 (define_insn "xxblend_<mode>"
   [(set (match_operand:VM3 0 "register_operand" "=wa")
@@ -932,7 +937,8 @@ (define_insn "xxblend_<mode>"
 		    UNSPEC_XXBLEND))]
   "TARGET_POWER10"
   "xxblendv<VM3_char> %x0,%x1,%x2,%x3"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")
+   (set_attr "prefixed" "always")])
 
 (define_expand "xxpermx"
   [(set (match_operand:V2DI 0 "register_operand" "+wa")
@@ -976,7 +982,8 @@ (define_insn "xxpermx_inst"
 		     UNSPEC_XXPERMX))]
   "TARGET_POWER10"
   "xxpermx %x0,%x1,%x2,%x3,%4"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")
+   (set_attr "prefixed" "always")])
 
 (define_expand "vstrir_<mode>"
   [(set (match_operand:VIshort 0 "altivec_register_operand")
@@ -998,7 +1005,7 @@ (define_insn "vstrir_code_<mode>"
 	   UNSPEC_VSTRIR))]
   "TARGET_POWER10"
   "vstri<wd>r %0,%1"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")])
 
 ;; This expands into same code as vstrir_<mode> followed by condition logic
 ;; so that a single vstribr. or vstrihr. or vstribl. or vstrihl. instruction
@@ -1028,7 +1035,8 @@ (define_insn "vstrir_p_code_<mode>"
 		   UNSPEC_VSTRIR))]
   "TARGET_POWER10"
   "vstri<wd>r. %0,%1"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")
+   (set_attr "dot" "yes")])
 
 (define_expand "vstril_<mode>"
   [(set (match_operand:VIshort 0 "altivec_register_operand")
@@ -1050,7 +1058,7 @@ (define_insn "vstril_code_<mode>"
 	   UNSPEC_VSTRIL))]
   "TARGET_POWER10"
   "vstri<wd>l %0,%1"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")])
 
 ;; This expands into same code as vstril_<mode> followed by condition logic
 ;; so that a single vstribr. or vstrihr. or vstribl. or vstrihl. instruction
@@ -1080,7 +1088,8 @@ (define_insn "vstril_p_code_<mode>"
 		   UNSPEC_VSTRIR))]
   "TARGET_POWER10"
   "vstri<wd>l. %0,%1"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")
+   (set_attr "dot" "yes")])
 
 ;; Fused multiply subtract 
 (define_insn "*altivec_vnmsubfp"
@@ -2779,7 +2788,7 @@ (define_insn "altivec_lvsl_reg"
 	UNSPEC_LVSL_REG))]
   "TARGET_ALTIVEC"
   "lvsl %0,0,%1"
-  [(set_attr "type" "vecload")])
+  [(set_attr "type" "vecperm")])
 
 (define_insn "altivec_lvsl_direct"
   [(set (match_operand:V16QI 0 "register_operand" "=v")
@@ -2787,7 +2796,7 @@ (define_insn "altivec_lvsl_direct"
 		      UNSPEC_LVSL))]
   "TARGET_ALTIVEC"
   "lvsl %0,%y1"
-  [(set_attr "type" "vecload")])
+  [(set_attr "type" "vecperm")])
 
 (define_expand "altivec_lvsr"
   [(use (match_operand:V16QI 0 "altivec_register_operand"))
@@ -2817,7 +2826,7 @@ (define_insn "altivec_lvsr_reg"
        UNSPEC_LVSR_REG))]
   "TARGET_ALTIVEC"
   "lvsr %0,0,%1"
-  [(set_attr "type" "vecload")])
+  [(set_attr "type" "vecperm")])
 
 (define_insn "altivec_lvsr_direct"
   [(set (match_operand:V16QI 0 "register_operand" "=v")
@@ -2825,7 +2834,7 @@ (define_insn "altivec_lvsr_direct"
 		      UNSPEC_LVSR))]
   "TARGET_ALTIVEC"
   "lvsr %0,%y1"
-  [(set_attr "type" "vecload")])
+  [(set_attr "type" "vecperm")])
 
 (define_expand "build_vector_mask_for_load"
   [(set (match_operand:V16QI 0 "register_operand")
@@ -3624,7 +3633,8 @@ (define_insn "xxeval"
 		     UNSPEC_XXEVAL))]
    "TARGET_POWER10"
    "xxeval %0,%1,%2,%3,%4"
-   [(set_attr "type" "vecsimple")])
+   [(set_attr "type" "vecperm")
+    (set_attr "prefixed" "always")])
 
 (define_expand "vec_unpacku_hi_v16qi"
   [(set (match_operand:V8HI 0 "register_operand" "=v")
@@ -4465,7 +4475,7 @@ (define_insn "vcfuged"
 	 UNSPEC_VCFUGED))]
    "TARGET_POWER10"
    "vcfuged %0,%1,%2"
-   [(set_attr "type" "vecsimple")])
+   [(set_attr "type" "crypto")])
 
 (define_insn "vclzdm"
   [(set (match_operand:V2DI 0 "altivec_register_operand" "=v")
@@ -4474,7 +4484,7 @@ (define_insn "vclzdm"
 	 UNSPEC_VCLZDM))]
    "TARGET_POWER10"
    "vclzdm %0,%1,%2"
-   [(set_attr "type" "vecsimple")])
+   [(set_attr "type" "crypto")])
 
 (define_insn "vctzdm"
   [(set (match_operand:V2DI 0 "altivec_register_operand" "=v")
@@ -4483,7 +4493,7 @@ (define_insn "vctzdm"
 	 UNSPEC_VCTZDM))]
    "TARGET_POWER10"
    "vctzdm %0,%1,%2"
-   [(set_attr "type" "vecsimple")])
+   [(set_attr "type" "crypto")])
 
 (define_insn "vpdepd"
   [(set (match_operand:V2DI 0 "altivec_register_operand" "=v")
@@ -4492,7 +4502,7 @@ (define_insn "vpdepd"
 	 UNSPEC_VPDEPD))]
    "TARGET_POWER10"
    "vpdepd %0,%1,%2"
-   [(set_attr "type" "vecsimple")])
+   [(set_attr "type" "crypto")])
 
 (define_insn "vpextd"
   [(set (match_operand:V2DI 0 "altivec_register_operand" "=v")
@@ -4501,7 +4511,7 @@ (define_insn "vpextd"
 	 UNSPEC_VPEXTD))]
    "TARGET_POWER10"
    "vpextd %0,%1,%2"
-   [(set_attr "type" "vecsimple")])
+   [(set_attr "type" "crypto")])
 
 (define_insn "vgnb"
   [(set (match_operand:DI 0 "register_operand" "=r")
@@ -4510,7 +4520,7 @@ (define_insn "vgnb"
          UNSPEC_VGNB))]
    "TARGET_POWER10"
    "vgnb %0,%1,%2"
-   [(set_attr "type" "vecsimple")])
+   [(set_attr "type" "crypto")])
 
 (define_insn "vclrlb"
   [(set (match_operand:V16QI 0 "altivec_register_operand" "=v")
@@ -4524,7 +4534,7 @@ (define_insn "vclrlb"
   else
     return "vclrrb %0,%1,%2";
 }
-   [(set_attr "type" "vecsimple")])
+   [(set_attr "type" "vecperm")])
 
 (define_insn "vclrrb"
   [(set (match_operand:V16QI 0 "altivec_register_operand" "=v")
@@ -4538,7 +4548,7 @@ (define_insn "vclrrb"
   else
     return "vclrlb %0,%1,%2";
 }
-   [(set_attr "type" "vecsimple")])
+   [(set_attr "type" "vecperm")])
 
 (define_expand "bcd<bcd_add_sub>_<code>"
   [(parallel [(set (reg:CCFP CR6_REGNO)
diff --git a/gcc/config/rs6000/dfp.md b/gcc/config/rs6000/dfp.md
index 8f822732bac..54841075051 100644
--- a/gcc/config/rs6000/dfp.md
+++ b/gcc/config/rs6000/dfp.md
@@ -139,7 +139,8 @@ (define_insn "extendddtd2"
 	(float_extend:TD (match_operand:DD 1 "gpc_reg_operand" "d")))]
   "TARGET_DFP"
   "dctqpq %0,%1"
-  [(set_attr "type" "dfp")])
+  [(set_attr "type" "dfp")
+   (set_attr "size" "128")])
 
 ;; The result of drdpq is an even/odd register pair with the converted
 ;; value in the even register and zero in the odd register.
@@ -153,6 +154,7 @@ (define_insn "trunctddd2"
   "TARGET_DFP"
   "drdpq %2,%1\;fmr %0,%2"
   [(set_attr "type" "dfp")
+   (set_attr "size" "128")
    (set_attr "length" "8")])
 
 (define_insn "trunctdsd2"
@@ -206,7 +208,8 @@ (define_insn "*cmp<mode>_internal1"
 		      (match_operand:DDTD 2 "gpc_reg_operand" "d")))]
   "TARGET_DFP"
   "dcmpu<q> %0,%1,%2"
-  [(set_attr "type" "dfp")])
+  [(set_attr "type" "dfp")
+   (set_attr "size" "<bits>")])
 
 (define_insn "floatdidd2"
   [(set (match_operand:DD 0 "gpc_reg_operand" "=d")
@@ -220,7 +223,8 @@ (define_insn "floatditd2"
 	(float:TD (match_operand:DI 1 "gpc_reg_operand" "d")))]
   "TARGET_DFP"
   "dcffixq %0,%1"
-  [(set_attr "type" "dfp")])
+  [(set_attr "type" "dfp")
+   (set_attr "size" "128")])
 
 ;; Convert a decimal64/128 to a decimal64/128 whose value is an integer.
 ;; This is the first stage of converting it to an integer type.
@@ -230,7 +234,8 @@ (define_insn "ftrunc<mode>2"
 	(fix:DDTD (match_operand:DDTD 1 "gpc_reg_operand" "d")))]
   "TARGET_DFP"
   "drintn<q>. 0,%0,%1,1"
-  [(set_attr "type" "dfp")])
+  [(set_attr "type" "dfp")
+   (set_attr "size" "<bits>")])
 
 ;; Convert a decimal64/128 whose value is an integer to an actual integer.
 ;; This is the second stage of converting decimal float to integer type.
@@ -240,7 +245,8 @@ (define_insn "fix<mode>di2"
 	(fix:DI (match_operand:DDTD 1 "gpc_reg_operand" "d")))]
   "TARGET_DFP"
   "dctfix<q> %0,%1"
-  [(set_attr "type" "dfp")])
+  [(set_attr "type" "dfp")
+   (set_attr "size" "<bits>")])
 \f
 ;; Decimal builtin support
 
@@ -262,7 +268,8 @@ (define_insn "dfp_ddedpd_<mode>"
 		     UNSPEC_DDEDPD))]
   "TARGET_DFP"
   "ddedpd<q> %1,%0,%2"
-  [(set_attr "type" "dfp")])
+  [(set_attr "type" "dfp")
+   (set_attr "size" "<bits>")])
 
 (define_insn "dfp_denbcd_<mode>"
   [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
@@ -271,7 +278,8 @@ (define_insn "dfp_denbcd_<mode>"
 		     UNSPEC_DENBCD))]
   "TARGET_DFP"
   "denbcd<q> %1,%0,%2"
-  [(set_attr "type" "dfp")])
+  [(set_attr "type" "dfp")
+   (set_attr "size" "<bits>")])
 
 (define_insn "dfp_dxex_<mode>"
   [(set (match_operand:DI 0 "gpc_reg_operand" "=d")
@@ -279,7 +287,8 @@ (define_insn "dfp_dxex_<mode>"
 		   UNSPEC_DXEX))]
   "TARGET_DFP"
   "dxex<q> %0,%1"
-  [(set_attr "type" "dfp")])
+  [(set_attr "type" "dfp")
+   (set_attr "size" "<bits>")])
 
 (define_insn "dfp_diex_<mode>"
   [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
@@ -288,7 +297,8 @@ (define_insn "dfp_diex_<mode>"
 		     UNSPEC_DXEX))]
   "TARGET_DFP"
   "diex<q> %0,%1,%2"
-  [(set_attr "type" "dfp")])
+  [(set_attr "type" "dfp")
+   (set_attr "size" "<bits>")])
 
 (define_expand "dfptstsfi_<code>_<mode>"
   [(set (match_dup 3)
@@ -327,7 +337,8 @@ (define_insn "*dfp_sgnfcnc_<mode>"
     operands[1] = GEN_INT (63);
   return "dtstsfi<q> %0,%1,%2";
 }
-  [(set_attr "type" "fp")])
+  [(set_attr "type" "fp")
+   (set_attr "size" "<bits>")])
 
 (define_insn "dfp_dscli_<mode>"
   [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
@@ -336,7 +347,8 @@ (define_insn "dfp_dscli_<mode>"
 		     UNSPEC_DSCLI))]
   "TARGET_DFP"
   "dscli<q> %0,%1,%2"
-  [(set_attr "type" "dfp")])
+  [(set_attr "type" "dfp")
+   (set_attr "size" "<bits>")])
 
 (define_insn "dfp_dscri_<mode>"
   [(set (match_operand:DDTD 0 "gpc_reg_operand" "=d")
@@ -345,4 +357,5 @@ (define_insn "dfp_dscri_<mode>"
 		     UNSPEC_DSCRI))]
   "TARGET_DFP"
   "dscri<q> %0,%1,%2"
-  [(set_attr "type" "dfp")])
+  [(set_attr "type" "dfp")
+   (set_attr "size" "<bits>")])
diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
index a3fd28bdd0a..43d6b618929 100644
--- a/gcc/config/rs6000/mma.md
+++ b/gcc/config/rs6000/mma.md
@@ -302,6 +302,7 @@ (define_insn_and_split "*movpoi"
   DONE;
 }
   [(set_attr "type" "vecload,vecstore,veclogical")
+   (set_attr "size" "256,256,*")
    (set_attr "length" "*,*,8")])
 
 \f
@@ -471,6 +472,7 @@ (define_insn "mma_<vvi4i4i8>"
   "TARGET_MMA"
   "<vvi4i4i8> %A0,%x1,%x2,%3,%4,%5"
   [(set_attr "type" "mma")
+   (set_attr "prefixed" "always")
    (set_attr "length" "8")])
 
 (define_insn "mma_<avvi4i4i8>"
@@ -485,6 +487,7 @@ (define_insn "mma_<avvi4i4i8>"
   "TARGET_MMA"
   "<avvi4i4i8> %A0,%x2,%x3,%4,%5,%6"
   [(set_attr "type" "mma")
+   (set_attr "prefixed" "always")
    (set_attr "length" "8")])
 
 (define_insn "mma_<vvi4i4i2>"
@@ -498,6 +501,7 @@ (define_insn "mma_<vvi4i4i2>"
   "TARGET_MMA"
   "<vvi4i4i2> %A0,%x1,%x2,%3,%4,%5"
   [(set_attr "type" "mma")
+   (set_attr "prefixed" "always")
    (set_attr "length" "8")])
 
 (define_insn "mma_<avvi4i4i2>"
@@ -512,6 +516,7 @@ (define_insn "mma_<avvi4i4i2>"
   "TARGET_MMA"
   "<avvi4i4i2> %A0,%x2,%x3,%4,%5,%6"
   [(set_attr "type" "mma")
+   (set_attr "prefixed" "always")
    (set_attr "length" "8")])
 
 (define_insn "mma_<vvi4i4>"
@@ -524,6 +529,7 @@ (define_insn "mma_<vvi4i4>"
   "TARGET_MMA"
   "<vvi4i4> %A0,%x1,%x2,%3,%4"
   [(set_attr "type" "mma")
+   (set_attr "prefixed" "always")
    (set_attr "length" "8")])
 
 (define_insn "mma_<avvi4i4>"
@@ -537,6 +543,7 @@ (define_insn "mma_<avvi4i4>"
   "TARGET_MMA"
   "<avvi4i4> %A0,%x2,%x3,%4,%5"
   [(set_attr "type" "mma")
+   (set_attr "prefixed" "always")
    (set_attr "length" "8")])
 
 (define_insn "mma_<pvi4i2>"
@@ -549,6 +556,7 @@ (define_insn "mma_<pvi4i2>"
   "TARGET_MMA"
   "<pvi4i2> %A0,%x1,%x2,%3,%4"
   [(set_attr "type" "mma")
+   (set_attr "prefixed" "always")
    (set_attr "length" "8")])
 
 (define_insn "mma_<apvi4i2>"
@@ -562,6 +570,7 @@ (define_insn "mma_<apvi4i2>"
   "TARGET_MMA"
   "<apvi4i2> %A0,%x2,%x3,%4,%5"
   [(set_attr "type" "mma")
+   (set_attr "prefixed" "always")
    (set_attr "length" "8")])
 
 (define_insn "mma_<vvi4i4i4>"
@@ -575,6 +584,7 @@ (define_insn "mma_<vvi4i4i4>"
   "TARGET_MMA"
   "<vvi4i4i4> %A0,%x1,%x2,%3,%4,%5"
   [(set_attr "type" "mma")
+   (set_attr "prefixed" "always")
    (set_attr "length" "8")])
 
 (define_insn "mma_<avvi4i4i4>"
@@ -589,4 +599,5 @@ (define_insn "mma_<avvi4i4i4>"
   "TARGET_MMA"
   "<avvi4i4i4> %A0,%x2,%x3,%4,%5,%6"
   [(set_attr "type" "mma")
+   (set_attr "prefixed" "always")
    (set_attr "length" "8")])
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 4d528a39a37..55c47140672 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -25752,7 +25752,7 @@ static bool next_insn_prefixed_p;
 void
 rs6000_final_prescan_insn (rtx_insn *insn, rtx [], int)
 {
-  next_insn_prefixed_p = (get_attr_prefixed (insn) != PREFIXED_NO);
+  next_insn_prefixed_p = (get_attr_prefixed (insn) == PREFIXED_YES);
   return;
 }
 
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index dc060143104..be18115f4bf 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -209,7 +209,7 @@ (define_attr "type"
 
 ;; What data size does this instruction work on?
 ;; This is used for insert, mul and others as necessary.
-(define_attr "size" "8,16,32,64,128" (const_string "32"))
+(define_attr "size" "8,16,32,64,128,256" (const_string "32"))
 
 ;; What is the insn_cost for this insn?  The target hook can still override
 ;; this.  For optimizing for size the "length" attribute is used instead.
@@ -264,13 +264,12 @@ (define_attr "var_shift" "no,yes"
 (define_attr "cannot_copy" "no,yes" (const_string "no"))
 
 
-;; Whether an insn is a prefixed insn, and an initial 'p' should be printed
-;; before the instruction.  A prefixed instruction has a prefix instruction
-;; word that extends the immediate value of the instructions from 12-16 bits to
-;; 34 bits.  The macro ASM_OUTPUT_OPCODE emits a leading 'p' for prefixed
-;; insns.  The default "length" attribute will also be adjusted by default to
-;; be 12 bytes.
-(define_attr "prefixed" "no,yes"
+;; Whether an insn is a prefixed insn.  A prefixed instruction has a prefix
+;; instruction word that extends the immediate value of the instructions from
+;; 12-16 bits to 34 bits.  The macro ASM_OUTPUT_OPCODE emits a leading 'p' for
+;; prefixed="yes" insns.  The default "length" attribute will also be adjusted
+;; by default to be 12 bytes.
+(define_attr "prefixed" "no,yes,always"
   (cond [(ior (match_test "!TARGET_PREFIXED")
 	      (match_test "!NONJUMP_INSN_P (insn)"))
 	 (const_string "no")
@@ -670,7 +669,8 @@ (define_mode_attr du_or_d [(QI    "du")
 
 ;; How many bits in this mode?
 (define_mode_attr bits [(QI "8") (HI "16") (SI "32") (DI "64")
-					   (SF "32") (DF "64")])
+					   (SF "32") (DF "64")
+					   	     (DD "64") (TD "128")])
 
 ; DImode bits
 (define_mode_attr dbits [(QI "56") (HI "48") (SI "32")])
@@ -2481,7 +2481,7 @@ (define_insn "cfuged"
 	 UNSPEC_CFUGED))]
    "TARGET_POWER10 && TARGET_64BIT"
    "cfuged %0,%1,%2"
-   [(set_attr "type" "integer")])
+   [(set_attr "type" "crypto")])
 
 (define_insn "cntlzdm"
   [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
@@ -2490,7 +2490,7 @@ (define_insn "cntlzdm"
 	 UNSPEC_CNTLZDM))]
    "TARGET_POWER10 && TARGET_POWERPC64"
    "cntlzdm %0,%1,%2"
-   [(set_attr "type" "integer")])
+   [(set_attr "type" "crypto")])
 
 (define_insn "cnttzdm"
   [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
@@ -2499,7 +2499,7 @@ (define_insn "cnttzdm"
 	 UNSPEC_CNTTZDM))]
    "TARGET_POWER10 && TARGET_POWERPC64"
    "cnttzdm %0,%1,%2"
-   [(set_attr "type" "integer")])
+   [(set_attr "type" "crypto")])
 
 (define_insn "pdepd"
   [(set (match_operand:DI 0 "register_operand" "=r")
@@ -2508,7 +2508,7 @@ (define_insn "pdepd"
 		   UNSPEC_PDEPD))]
    "TARGET_POWER10 && TARGET_POWERPC64"
    "pdepd %0,%1,%2"
-   [(set_attr "type" "integer")])
+   [(set_attr "type" "crypto")])
 
 (define_insn "pextd"
   [(set (match_operand:DI 0 "register_operand" "=r")
@@ -2517,7 +2517,7 @@ (define_insn "pextd"
 		   UNSPEC_PEXTD))]
    "TARGET_POWER10 && TARGET_POWERPC64"
    "pextd %0,%1,%2"
-   [(set_attr "type" "integer")])
+   [(set_attr "type" "crypto")])
 
 (define_insn "cmpb<mode>3"
   [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
@@ -2617,7 +2617,7 @@ (define_insn_and_split "bswaphi2_reg"
   operands[4] = simplify_gen_subreg (SImode, operands[1], HImode, 0);
 }
   [(set_attr "length" "*,12,*")
-   (set_attr "type" "shift,*,vecperm")
+   (set_attr "type" "vecperm,*,vecperm")
    (set_attr "isa" "p10,*,p9v")])
 
 ;; We are always BITS_BIG_ENDIAN, so the bit positions below in
@@ -2649,7 +2649,7 @@ (define_insn_and_split "bswapsi2_reg"
 			(const_int -256))))]
   ""
   [(set_attr "length" "4,12,4")
-   (set_attr "type" "shift,*,vecperm")
+   (set_attr "type" "vecperm,*,vecperm")
    (set_attr "isa" "p10,*,p9v")])
 
 ;; On systems with LDBRX/STDBRX generate the loads/stores directly, just like
@@ -2721,7 +2721,7 @@ (define_insn "bswapdi2_brd"
   "@
    brd %0,%1
    xxbrd %x0,%x1"
-  [(set_attr "type" "shift,vecperm")
+  [(set_attr "type" "vecperm,vecperm")
    (set_attr "isa" "p10,p9v")])
 
 (define_insn "bswapdi2_reg"
@@ -5242,7 +5242,7 @@ (define_insn "setbc_<un>signed_<GPR:mode>"
 			 (const_int 0)]))]
   "TARGET_POWER10"
   "setbc %0,%j1"
-  [(set_attr "type" "isel")])
+  [(set_attr "type" "integer")])
 
 (define_insn "*setbcr_<un>signed_<GPR:mode>"
   [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
@@ -5251,7 +5251,7 @@ (define_insn "*setbcr_<un>signed_<GPR:mode>"
 			 (const_int 0)]))]
   "TARGET_POWER10"
   "setbcr %0,%j1"
-  [(set_attr "type" "isel")])
+  [(set_attr "type" "integer")])
 
 ; Set Negative Boolean Condition (Reverse)
 (define_insn "*setnbc_<un>signed_<GPR:mode>"
@@ -5261,7 +5261,7 @@ (define_insn "*setnbc_<un>signed_<GPR:mode>"
 			 (const_int 0)])))]
   "TARGET_POWER10"
   "setnbc %0,%j1"
-  [(set_attr "type" "isel")])
+  [(set_attr "type" "integer")])
 
 (define_insn "*setnbcr_<un>signed_<GPR:mode>"
   [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
@@ -5270,7 +5270,7 @@ (define_insn "*setnbcr_<un>signed_<GPR:mode>"
 			 (const_int 0)])))]
   "TARGET_POWER10"
   "setnbcr %0,%j1"
-  [(set_attr "type" "isel")])
+  [(set_attr "type" "integer")])
 
 ;; Floating point conditional move
 (define_expand "mov<mode>cc"
diff --git a/gcc/config/rs6000/sync.md b/gcc/config/rs6000/sync.md
index 5ad88806818..b07b2e86aae 100644
--- a/gcc/config/rs6000/sync.md
+++ b/gcc/config/rs6000/sync.md
@@ -131,6 +131,7 @@ (define_insn "load_quadpti"
    && !reg_mentioned_p (operands[0], operands[1])"
   "lq %0,%1"
   [(set_attr "type" "load")
+   (set_attr "size" "128")
    (set (attr "prefixed") (if_then_else (match_test "TARGET_PREFIXED")
 					(const_string "yes")
 					(const_string "no")))])
@@ -205,6 +206,7 @@ (define_insn "store_quadpti"
   "TARGET_SYNC_TI"
   "stq %1,%0"
   [(set_attr "type" "store")
+   (set_attr "size" "128")
    (set (attr "prefixed") (if_then_else (match_test "TARGET_PREFIXED")
 					(const_string "yes")
 					(const_string "no")))])
@@ -333,7 +335,8 @@ (define_insn "load_lockedpti"
    && !reg_mentioned_p (operands[0], operands[1])
    && quad_int_reg_operand (operands[0], PTImode)"
   "lqarx %0,%y1"
-  [(set_attr "type" "load_l")])
+  [(set_attr "type" "load_l")
+   (set_attr "size" "128")])
 
 (define_insn "store_conditional<mode>"
   [(set (match_operand:CC 0 "cc_reg_operand" "=x")
@@ -394,7 +397,8 @@ (define_insn "store_conditionalpti"
 	(match_operand:PTI 2 "quad_int_reg_operand" "r"))]
   "TARGET_SYNC_TI && quad_int_reg_operand (operands[2], PTImode)"
   "stqcx. %2,%y1"
-  [(set_attr "type" "store_c")])
+  [(set_attr "type" "store_c")
+   (set_attr "size" "128")])
 
 (define_expand "atomic_compare_and_swap<mode>"
   [(match_operand:SI 0 "int_reg_operand")		;; bool out
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index d6347dba149..1b86165090d 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -2032,7 +2032,7 @@ (define_insn "*xvtlsbb_internal"
 	 UNSPEC_XVTLSBB))]
   "TARGET_POWER10"
   "xvtlsbb %0,%x1"
-  [(set_attr "type" "logical")])
+  [(set_attr "type" "veccmp")])
 
 ;; Vector Test Least Significant Bit by Byte
 ;; for the implementation of the builtin
@@ -3111,7 +3111,7 @@ (define_insn "xxgenpcvm_<mode>_internal"
 	 UNSPEC_XXGENPCV))]
     "TARGET_POWER10 && TARGET_64BIT"
     "xxgenpcv<wd>m %x0,%1,%2"
-    [(set_attr "type" "vecsimple")])
+    [(set_attr "type" "vecperm")])
 
 (define_expand "xxgenpcvm_<mode>"
   [(use (match_operand:VSX_EXTRACT_I4 0 "register_operand"))
@@ -3913,7 +3913,7 @@ (define_insn "vextractl<mode>_internal"
 		     UNSPEC_EXTRACTL))]
   "TARGET_POWER10"
   "vext<du_or_d><wd>vlx %0,%1,%2,%3"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")])
 
 (define_expand "vextractr<mode>"
   [(set (match_operand:V2DI 0 "altivec_register_operand")
@@ -3943,7 +3943,7 @@ (define_insn "vextractr<mode>_internal"
 		     UNSPEC_EXTRACTR))]
   "TARGET_POWER10"
   "vext<du_or_d><wd>vrx %0,%1,%2,%3"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")])
 
 (define_expand "vinsertvl_<mode>"
   [(set (match_operand:VI2 0 "altivec_register_operand")
@@ -3970,7 +3970,7 @@ (define_insn "vinsertvl_internal_<mode>"
 		      UNSPEC_INSERTL))]
   "TARGET_POWER10"
   "vins<wd>vlx %0,%1,%2"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")])
 
 (define_expand "vinsertvr_<mode>"
   [(set (match_operand:VI2 0 "altivec_register_operand")
@@ -3997,7 +3997,7 @@ (define_insn "vinsertvr_internal_<mode>"
 		      UNSPEC_INSERTR))]
   "TARGET_POWER10"
   "vins<wd>vrx %0,%1,%2"
-  [(set_attr "type" "vecsimple")])
+  [(set_attr "type" "vecperm")])
 
 (define_expand "vinsertgl_<mode>"
   [(set (match_operand:VI2 0 "altivec_register_operand")
@@ -4024,7 +4024,7 @@ (define_insn "vinsertgl_internal_<mode>"
 		     UNSPEC_INSERTL))]
  "TARGET_POWER10"
  "vins<wd>lx %0,%1,%2"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecperm")])
 
 (define_expand "vinsertgr_<mode>"
   [(set (match_operand:VI2 0 "altivec_register_operand")
@@ -4051,7 +4051,7 @@ (define_insn "vinsertgr_internal_<mode>"
 		 UNSPEC_INSERTR))]
  "TARGET_POWER10"
  "vins<wd>rx %0,%1,%2"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecperm")])
 
 (define_expand "vreplace_elt_<mode>"
   [(set (match_operand:REPLACE_ELT 0 "register_operand")
@@ -4100,7 +4100,7 @@ (define_insn "vreplace_elt_<mode>_inst"
 		      UNSPEC_REPLACE_ELT))]
  "TARGET_POWER10"
  "vins<REPLACE_ELT_char> %0,%2,%3"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecperm")])
 
 ;; VSX_EXTRACT optimizations
 ;; Optimize double d = (double) vec_extract (vi, <n>)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, rs6000] Update instruction attributes for Power10
  2020-11-04 20:42 [PATCH, rs6000] Update instruction attributes for Power10 Pat Haugen
@ 2020-11-05 22:32 ` will schmidt
  2020-11-06 16:46   ` Pat Haugen
  2020-11-09 21:54 ` Segher Boessenkool
  1 sibling, 1 reply; 6+ messages in thread
From: will schmidt @ 2020-11-05 22:32 UTC (permalink / raw)
  To: Pat Haugen, GCC Patches; +Cc: Bill Schmidt, David Edelsohn, Segher Boessenkool

On Wed, 2020-11-04 at 14:42 -0600, Pat Haugen via Gcc-patches wrote:
> Update instruction attributes for Power10.
> 
> 
> This patch updates the type/prefixed/dot/size attributes for various new instructions (and a couple existing that were incorrect) in preparation for the Power10 scheduling patch that will be following.
> 
> Bootstrap/regtest on powerpc64le (Power8/Power10) with no new regressions. Ok for trunk?
> 
> -Pat
> 
> 
> 2020-11-04  Pat Haugen  <pthaugen@linux.ibm.com>
> 
> gcc/
> 	* config/rs6000/altivec.md (vs<SLDB_lr>db_<mode>, xxspltiw_v4si,
> 	xxspltiw_v4sf_inst, xxspltidp_v2df_inst, xxsplti32dx_v4si_inst,
> 	xxsplti32dx_v4sf_inst, xxblend_<mode>, xxpermx_inst,
> 	vstrir_code_<mode>, vstrir_p_code_<mode>, vstril_code_<mode>,
> 	vstril_p_code_<mode>, altivec_lvsl_reg, altivec_lvsl_direct,
> 	altivec_lvsr_reg, altivec_lvsr_direct, xxeval, vcfuged, vclzdm,
> 	vctzdm, vpdepd, vpextd, vgnb, vclrlb, vclrrb): Update instruction
> 	attributes for Power10.
> 	* config/rs6000/dfp.md (extendddtd2, trunctddd2, *cmp<mode>_internal1,
> 	floatditd2, ftrunc<mode>2, fix<mode>di2, dfp_ddedpd_<mode>,
> 	dfp_denbcd_<mode>, dfp_dxex_<mode>, dfp_diex_<mode>,
> 	*dfp_sgnfcnc_<mode>, dfp_dscli_<mode>, dfp_dscri_<mode>): Likewise.
> 	* config/rs6000/mma.md (*movpoi, mma_<vvi4i4i8>, mma_<avvi4i4i8>,
> 	mma_<vvi4i4i2>, mma_<avvi4i4i2>, mma_<vvi4i4>, mma_<avvi4i4>,
> 	mma_<pvi4i2>, mma_<apvi4i2>, mma_<vvi4i4i4>, mma_<avvi4i4i4>):
> 	Likewise.
> 	* config/rs6000/rs6000.c (rs6000_final_prescan_insn): Only add 'p' for
> 	PREFIXED_YES.

The code change reads as roughly 
- next_insns_prefixed_p != PREFIXED_NO

+ next_insn_prefixed_p == PREFIXED_YES"

So just an inversion of the logic? I don't obviously see the 'p' impact
there.


> 	* config/rs6000/rs6000.md (define_attr "size"): Add 256.
> 	(define_attr "prefixed"): Add 'always'.
> 	(define_mode_attr bits): Add DD/TD modes.
> 	(cfuged, cntlzdm, cnttzdm, pdepd, pextd, bswaphi2_reg, bswapsi2_reg,
> 	bswapdi2_brd, setbc_<un>signed_<GPR:mode>,
> 	*setbcr_<un>signed_<GPR:mode>, *setnbc_<un>signed_<GPR:mode>,
> 	*setnbcr_<un>signed_<GPR:mode>): Update instruction attributes for
> 	Power10.

ok.  (assuming the assorted 'integer' -> 'crypto' changes are correct,
of course).  

> 	* config/rs6000/sync.md (load_quadpti, store_quadpti, load_lockedpti,
> 	store_conditionalpti): Update instruction attributes for Power10.
> 	* config/rs6000/vsx.md (*xvtlsbb_internal, xxgenpcvm_<mode>_internal,
> 	vextractl<mode>_internal, vextractr<mode>_internal,
> 	vinsertvl_internal_<mode>, vinsertvr_internal_<mode>,
> 	vinsertgl_internal_<mode>, vinsertgr_internal_<mode>,
> 	vreplace_elt_<mode>_inst): Likewise.


lgtm, 
thanks
-Will

> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, rs6000] Update instruction attributes for Power10
  2020-11-05 22:32 ` will schmidt
@ 2020-11-06 16:46   ` Pat Haugen
  2020-11-06 17:56     ` will schmidt
  2020-11-09 20:32     ` Segher Boessenkool
  0 siblings, 2 replies; 6+ messages in thread
From: Pat Haugen @ 2020-11-06 16:46 UTC (permalink / raw)
  To: will schmidt, GCC Patches
  Cc: Bill Schmidt, David Edelsohn, Segher Boessenkool

On 11/5/20 4:32 PM, will schmidt wrote:
> On Wed, 2020-11-04 at 14:42 -0600, Pat Haugen via Gcc-patches wrote:
>> 	* config/rs6000/rs6000.c (rs6000_final_prescan_insn): Only add 'p' for
>> 	PREFIXED_YES.
> 
> The code change reads as roughly 
> - next_insn_prefixed_p != PREFIXED_NO
> 
> + next_insn_prefixed_p == PREFIXED_YES"
> 
> So just an inversion of the logic? I don't obviously see the 'p' impact
> there.
> 
It's no longer an inversion of the logic since I added a PREFIXED_ALWAYS value. 'next_insn_prefixed' is used by rs6000_final_prescan_insn() to determine whether an insn mnemonic needs a 'p' prefix. We want it set for PREFIXED_YES, but not for PREFIXED_NO or PREFIXED_ALWAYS.

> 
>> 	* config/rs6000/rs6000.md (define_attr "size"): Add 256.
>> 	(define_attr "prefixed"): Add 'always'.
>> 	(define_mode_attr bits): Add DD/TD modes.
>> 	(cfuged, cntlzdm, cnttzdm, pdepd, pextd, bswaphi2_reg, bswapsi2_reg,
>> 	bswapdi2_brd, setbc_<un>signed_<GPR:mode>,
>> 	*setbcr_<un>signed_<GPR:mode>, *setnbc_<un>signed_<GPR:mode>,
>> 	*setnbcr_<un>signed_<GPR:mode>): Update instruction attributes for
>> 	Power10.
> 
> ok.  (assuming the assorted 'integer' -> 'crypto' changes are correct,
> of course).  
> 
Yes, crypto represents the correct pipe the insns are executed on.

Thanks for the review,
Pat


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, rs6000] Update instruction attributes for Power10
  2020-11-06 16:46   ` Pat Haugen
@ 2020-11-06 17:56     ` will schmidt
  2020-11-09 20:32     ` Segher Boessenkool
  1 sibling, 0 replies; 6+ messages in thread
From: will schmidt @ 2020-11-06 17:56 UTC (permalink / raw)
  To: Pat Haugen, GCC Patches; +Cc: Bill Schmidt, David Edelsohn, Segher Boessenkool

On Fri, 2020-11-06 at 10:46 -0600, Pat Haugen wrote:
> On 11/5/20 4:32 PM, will schmidt wrote:
> > On Wed, 2020-11-04 at 14:42 -0600, Pat Haugen via Gcc-patches
> > wrote:
> > > 	* config/rs6000/rs6000.c (rs6000_final_prescan_insn): Only add
> > > 'p' for
> > > 	PREFIXED_YES.
> > 
> > The code change reads as roughly 
> > - next_insn_prefixed_p != PREFIXED_NO
> > 
> > + next_insn_prefixed_p == PREFIXED_YES"
> > 
> > So just an inversion of the logic? I don't obviously see the 'p'
> > impact
> > there.
> > 
> 
> It's no longer an inversion of the logic since I added a
> PREFIXED_ALWAYS value. 'next_insn_prefixed' is used by
> rs6000_final_prescan_insn() to determine whether an insn mnemonic
> needs a 'p' prefix. We want it set for PREFIXED_YES, but not for
> PREFIXED_NO or PREFIXED_ALWAYS.

Ok.  So the next_insn_prefixed_p indicates whether the instruction
has/gets/needs a p prefix.  gotcha.  thanks for clarifying.  :-)

thanks
-will


> 
> > 
> > > 	* config/rs6000/rs6000.md (define_attr "size"): Add 256.
> > > 	(define_attr "prefixed"): Add 'always'.
> > > 	(define_mode_attr bits): Add DD/TD modes.
> > > 	(cfuged, cntlzdm, cnttzdm, pdepd, pextd, bswaphi2_reg,
> > > bswapsi2_reg,
> > > 	bswapdi2_brd, setbc_<un>signed_<GPR:mode>,
> > > 	*setbcr_<un>signed_<GPR:mode>, *setnbc_<un>signed_<GPR:mode>,
> > > 	*setnbcr_<un>signed_<GPR:mode>): Update instruction attributes
> > > for
> > > 	Power10.
> > 
> > ok.  (assuming the assorted 'integer' -> 'crypto' changes are
> > correct,
> > of course).  
> > 
> 
> Yes, crypto represents the correct pipe the insns are executed on.
> 
> Thanks for the review,
> Pat
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, rs6000] Update instruction attributes for Power10
  2020-11-06 16:46   ` Pat Haugen
  2020-11-06 17:56     ` will schmidt
@ 2020-11-09 20:32     ` Segher Boessenkool
  1 sibling, 0 replies; 6+ messages in thread
From: Segher Boessenkool @ 2020-11-09 20:32 UTC (permalink / raw)
  To: Pat Haugen; +Cc: will schmidt, GCC Patches, Bill Schmidt, David Edelsohn

On Fri, Nov 06, 2020 at 10:46:43AM -0600, Pat Haugen wrote:
> On 11/5/20 4:32 PM, will schmidt wrote:
> > On Wed, 2020-11-04 at 14:42 -0600, Pat Haugen via Gcc-patches wrote:
> >> 	* config/rs6000/rs6000.c (rs6000_final_prescan_insn): Only add 'p' for
> >> 	PREFIXED_YES.
> > 
> > The code change reads as roughly 
> > - next_insn_prefixed_p != PREFIXED_NO
> > 
> > + next_insn_prefixed_p == PREFIXED_YES"
> > 
> > So just an inversion of the logic? I don't obviously see the 'p' impact
> > there.
> > 
> It's no longer an inversion of the logic since I added a PREFIXED_ALWAYS value. 'next_insn_prefixed' is used by rs6000_final_prescan_insn() to determine whether an insn mnemonic needs a 'p' prefix. We want it set for PREFIXED_YES, but not for PREFIXED_NO or PREFIXED_ALWAYS.

Another way to do this is to do like maybe_var_shift does: things you
mask maybe_prefixed=yes get their prefixed attribute set to no or yes
depending on their operands, and for things where we always want to
set the prefixed attribute we can do just that. I don't know what reads
better / is more clear, and/or is more work to write (the schemes are
semantically identical).



Segher

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, rs6000] Update instruction attributes for Power10
  2020-11-04 20:42 [PATCH, rs6000] Update instruction attributes for Power10 Pat Haugen
  2020-11-05 22:32 ` will schmidt
@ 2020-11-09 21:54 ` Segher Boessenkool
  1 sibling, 0 replies; 6+ messages in thread
From: Segher Boessenkool @ 2020-11-09 21:54 UTC (permalink / raw)
  To: Pat Haugen; +Cc: GCC Patches, David Edelsohn, Peter Bergner, Bill Schmidt

On Wed, Nov 04, 2020 at 02:42:39PM -0600, Pat Haugen wrote:
> This patch updates the type/prefixed/dot/size attributes for various new instructions (and a couple existing that were incorrect) in preparation for the Power10 scheduling patch that will be following.

> --- a/gcc/config/rs6000/altivec.md
> +++ b/gcc/config/rs6000/altivec.md
> @@ -819,7 +819,7 @@ (define_insn "vs<SLDB_lr>db_<mode>"
>  	      VSHIFT_DBL_LR))]
>    "TARGET_POWER10"
>    "vs<SLDB_lr>dbi %0,%1,%2,%3"
> -  [(set_attr "type" "vecsimple")])
> +  [(set_attr "type" "vecperm")])

Is that such a good type for this?  I know the vsl etc. insns use it as
well, but :-)

These insns use the PM pipe on p9, which is documented as "Permute /
128b FX".  So maybe something like that can be a better name?

(Just food for thought.)

> @@ -827,7 +827,8 @@ (define_insn "xxspltiw_v4si"
>  		     UNSPEC_XXSPLTIW))]
>   "TARGET_POWER10"
>   "xxspltiw %x0,%1"
> - [(set_attr "type" "vecsimple")])
> + [(set_attr "type" "vecperm")
> +  (set_attr "prefixed" "always")])

Like I said in the other thread, you could have a "maybe_prefixed"
attribute (which you use on pretty much all existing ones), and only if
that is set the "prefixed" attribute is set to "yes" or "no"
automatically.  And things like this that always want it set can just do
so.  The code that decides whether to prefix the mnemonic with a "p" can
then just look if "maybe_prefixed" is "yes".

I don't know what is nicer, such a scheme or what you have here.  Will
ran into a pitfall already, so dunno.

> @@ -1080,7 +1088,8 @@ (define_insn "vstril_p_code_<mode>"
>  		   UNSPEC_VSTRIR))]
>    "TARGET_POWER10"
>    "vstri<wd>l. %0,%1"
> -  [(set_attr "type" "vecsimple")])
> +  [(set_attr "type" "vecperm")
> +   (set_attr "dot" "yes")])

"dot" is documented as
;; Is this instruction record form ("dot", signed compare to 0, writing CR0)?
which this insn doesn't do (it doesn't do a comparison, and it writes to
CR6).  It is fine to have this attribute if that is useful, but then
change the docs?  (FP insns write CR1 btw; that is all three fields that
record form insns can write).

>  ;; Fused multiply subtract 
>  (define_insn "*altivec_vnmsubfp"
> @@ -2779,7 +2788,7 @@ (define_insn "altivec_lvsl_reg"
>  	UNSPEC_LVSL_REG))]
>    "TARGET_ALTIVEC"
>    "lvsl %0,0,%1"
> -  [(set_attr "type" "vecload")])
> +  [(set_attr "type" "vecperm")])

That is not correct on older processors (7400, 970, etc.), and it even
matters there (you can have only one insn per pipe in each cycle).  Not
that that will matter much for GCC 11, but perhaps we can make the model
better for new CPUs without making it worse for older ones :-)

> @@ -4465,7 +4475,7 @@ (define_insn "vcfuged"
>  	 UNSPEC_VCFUGED))]
>     "TARGET_POWER10"
>     "vcfuged %0,%1,%2"
> -   [(set_attr "type" "vecsimple")])
> +   [(set_attr "type" "crypto")])
>  
>  (define_insn "vclzdm"
>    [(set (match_operand:V2DI 0 "altivec_register_operand" "=v")
> @@ -4474,7 +4484,7 @@ (define_insn "vclzdm"
>  	 UNSPEC_VCLZDM))]
>     "TARGET_POWER10"
>     "vclzdm %0,%1,%2"
> -   [(set_attr "type" "vecsimple")])
> +   [(set_attr "type" "crypto")])

Yeah that is a pretty strange type for what these insn do.

We already have the name "veccomplex", but a name like that would be
fitting perhaps.

> @@ -345,4 +357,5 @@ (define_insn "dfp_dscri_<mode>"
>  		     UNSPEC_DSCRI))]
>    "TARGET_DFP"
>    "dscri<q> %0,%1,%2"
> -  [(set_attr "type" "dfp")])
> +  [(set_attr "type" "dfp")
> +   (set_attr "size" "<bits>")])

All the DFP size changes are fine.

> diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
> index a3fd28bdd0a..43d6b618929 100644
> --- a/gcc/config/rs6000/mma.md
> +++ b/gcc/config/rs6000/mma.md
> @@ -302,6 +302,7 @@ (define_insn_and_split "*movpoi"
>    DONE;
>  }
>    [(set_attr "type" "vecload,vecstore,veclogical")
> +   (set_attr "size" "256,256,*")
>     (set_attr "length" "*,*,8")])

And this, too.

> @@ -589,4 +599,5 @@ (define_insn "mma_<avvi4i4i4>"
>    "TARGET_MMA"
>    "<avvi4i4i4> %A0,%x2,%x3,%4,%5,%6"
>    [(set_attr "type" "mma")
> +   (set_attr "prefixed" "always")
>     (set_attr "length" "8")])

And all the prefixed ones, too.

> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 4d528a39a37..55c47140672 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -25752,7 +25752,7 @@ static bool next_insn_prefixed_p;
>  void
>  rs6000_final_prescan_insn (rtx_insn *insn, rtx [], int)
>  {
> -  next_insn_prefixed_p = (get_attr_prefixed (insn) != PREFIXED_NO);
> +  next_insn_prefixed_p = (get_attr_prefixed (insn) == PREFIXED_YES);
>    return;
>  }

Add a comment here?  "always" is not surprising in many places, but here
it is.

> -;; Whether an insn is a prefixed insn, and an initial 'p' should be printed
> -;; before the instruction.  A prefixed instruction has a prefix instruction
> -;; word that extends the immediate value of the instructions from 12-16 bits to
> -;; 34 bits.  The macro ASM_OUTPUT_OPCODE emits a leading 'p' for prefixed
> -;; insns.  The default "length" attribute will also be adjusted by default to
> -;; be 12 bytes.
> -(define_attr "prefixed" "no,yes"
> +;; Whether an insn is a prefixed insn.  A prefixed instruction has a prefix
> +;; instruction word that extends the immediate value of the instructions from
> +;; 12-16 bits to 34 bits.  The macro ASM_OUTPUT_OPCODE emits a leading 'p' for
> +;; prefixed="yes" insns.  The default "length" attribute will also be adjusted
> +;; by default to be 12 bytes.

In xxspltiw it is only 32 bits, not 34.  This comment really only
describes the 8LS-form prefix insns.

> @@ -2517,7 +2517,7 @@ (define_insn "pextd"
>  		   UNSPEC_PEXTD))]
>     "TARGET_POWER10 && TARGET_POWERPC64"
>     "pextd %0,%1,%2"
> -   [(set_attr "type" "integer")])
> +   [(set_attr "type" "crypto")])

Same as above, but for integer instead of vector.

>  (define_insn "bswapdi2_reg"
> @@ -5242,7 +5242,7 @@ (define_insn "setbc_<un>signed_<GPR:mode>"
>  			 (const_int 0)]))]
>    "TARGET_POWER10"
>    "setbc %0,%j1"
> -  [(set_attr "type" "isel")])
> +  [(set_attr "type" "integer")])

Why?  Are these insns executed differently than the isel insn?  They do
take a CR field as input, which none of the integer isns do.

> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -2032,7 +2032,7 @@ (define_insn "*xvtlsbb_internal"
>  	 UNSPEC_XVTLSBB))]
>    "TARGET_POWER10"
>    "xvtlsbb %0,%x1"
> -  [(set_attr "type" "logical")])
> +  [(set_attr "type" "veccmp")])

Yup.

Maybe split the patch into a few themes?  Some themes are much easier
than others to review, are much more obvious; and they are all
independent anyway?


Segher

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-09 21:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-04 20:42 [PATCH, rs6000] Update instruction attributes for Power10 Pat Haugen
2020-11-05 22:32 ` will schmidt
2020-11-06 16:46   ` Pat Haugen
2020-11-06 17:56     ` will schmidt
2020-11-09 20:32     ` Segher Boessenkool
2020-11-09 21:54 ` 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).