* [PATCH, rs6000 V2] Update "prefix" attribute for Power10 [PR99133]
@ 2021-03-17 20:49 Pat Haugen
2021-03-18 16:33 ` will schmidt
2021-03-23 16:50 ` Segher Boessenkool
0 siblings, 2 replies; 4+ messages in thread
From: Pat Haugen @ 2021-03-17 20:49 UTC (permalink / raw)
To: GCC Patches
Cc: Segher Boessenkool, David Edelsohn, Peter Bergner, Bill Schmidt
[-- Attachment #1: Type: text/plain, Size: 1423 bytes --]
Update prefixed attribute for Power10.
This patch creates a new attribute, prepend_prefixed_insn, which is used to mark
those instructions that are prefixed and need to have a 'p' prepended to their
mnemonic at asm emit time. The existing "prefix" attribute is now used to mark
all instructions that are prefixed form.
Bootstrap/regtest on powerpc64le (Power10) and powerpc64 (Power8 32/64) with no
new regressions. Ok for trunk?
-Pat
2021-03-17 Pat Haugen <pthaugen@linux.ibm.com>
gcc/
PR target/99133
* config/rs6000/altivec.md (xxspltiw_v4si, xxspltiw_v4sf_inst,
xxspltidp_v2df_inst, xxsplti32dx_v4si_inst, xxsplti32dx_v4sf_inst,
xxblend_<mode>, xxpermx_inst, xxeval): Mark prefixed.
* config/rs6000/mma.md (mma_<vvi4i4i8>, mma_<avvi4i4i8>,
mma_<vvi4i4i2>, mma_<avvi4i4i2>, mma_<vvi4i4>, mma_<avvi4i4>,
mma_<pvi4i2>, mma_<apvi4i2>, mma_<vvi4i4i4>, mma_<avvi4i4i4>):
Likewise.
* config/rs6000/pcrel-opt.md: Adjust attribute name.
* config/rs6000/rs6000.c (rs6000_final_prescan_insn): Adjust test.
* config/rs6000/rs6000.md (define_attr "prepend_prefixed_insn"): New.
(define_attr "prefixed"): Update initializer.
(*tls_gd_pcrel<bits>, *tls_ld_pcrel<bits>, tls_dtprel_<bits>,
tls_tprel_<bits>, *tls_got_tprel_pcrel_<bits>, *pcrel_local_addr,
*pcrel_extern_addr, stack_protect_setdi, stack_protect_testdi):
Adjust attribute name.
* config/rs6000/sync.md (load_quadpti, store_quadpti): Likewise.
[-- Attachment #2: p10_prefixed.diff --]
[-- Type: text/plain, Size: 13660 bytes --]
diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
index 27a269b9e72..21f1cc6f15b 100644
--- a/gcc/config/rs6000/altivec.md
+++ b/gcc/config/rs6000/altivec.md
@@ -826,7 +826,8 @@ (define_insn "xxspltiw_v4si"
UNSPEC_XXSPLTIW))]
"TARGET_POWER10"
"xxspltiw %x0,%1"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecsimple")
+ (set_attr "prefixed" "yes")])
(define_expand "xxspltiw_v4sf"
[(set (match_operand:V4SF 0 "register_operand" "=wa")
@@ -845,7 +846,8 @@ (define_insn "xxspltiw_v4sf_inst"
UNSPEC_XXSPLTIW))]
"TARGET_POWER10"
"xxspltiw %x0,%1"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecsimple")
+ (set_attr "prefixed" "yes")])
(define_expand "xxspltidp_v2df"
[(set (match_operand:V2DF 0 "register_operand" )
@@ -864,7 +866,8 @@ (define_insn "xxspltidp_v2df_inst"
UNSPEC_XXSPLTID))]
"TARGET_POWER10"
"xxspltidp %x0,%1"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecsimple")
+ (set_attr "prefixed" "yes")])
(define_expand "xxsplti32dx_v4si"
[(set (match_operand:V4SI 0 "register_operand" "=wa")
@@ -893,7 +896,8 @@ (define_insn "xxsplti32dx_v4si_inst"
UNSPEC_XXSPLTI32DX))]
"TARGET_POWER10"
"xxsplti32dx %x0,%2,%3"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecsimple")
+ (set_attr "prefixed" "yes")])
(define_expand "xxsplti32dx_v4sf"
[(set (match_operand:V4SF 0 "register_operand" "=wa")
@@ -921,7 +925,8 @@ (define_insn "xxsplti32dx_v4sf_inst"
UNSPEC_XXSPLTI32DX))]
"TARGET_POWER10"
"xxsplti32dx %x0,%2,%3"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecsimple")
+ (set_attr "prefixed" "yes")])
(define_insn "xxblend_<mode>"
[(set (match_operand:VM3 0 "register_operand" "=wa")
@@ -931,7 +936,8 @@ (define_insn "xxblend_<mode>"
UNSPEC_XXBLEND))]
"TARGET_POWER10"
"xxblendv<VM3_char> %x0,%x1,%x2,%x3"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecsimple")
+ (set_attr "prefixed" "yes")])
(define_expand "xxpermx"
[(set (match_operand:V2DI 0 "register_operand" "+wa")
@@ -975,7 +981,8 @@ (define_insn "xxpermx_inst"
UNSPEC_XXPERMX))]
"TARGET_POWER10"
"xxpermx %x0,%x1,%x2,%x3,%4"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecsimple")
+ (set_attr "prefixed" "yes")])
(define_expand "vstrir_<mode>"
[(set (match_operand:VIshort 0 "altivec_register_operand")
@@ -3623,7 +3630,8 @@ (define_insn "xxeval"
UNSPEC_XXEVAL))]
"TARGET_POWER10"
"xxeval %0,%1,%2,%3,%4"
- [(set_attr "type" "vecsimple")])
+ [(set_attr "type" "vecsimple")
+ (set_attr "prefixed" "yes")])
(define_expand "vec_unpacku_hi_v16qi"
[(set (match_operand:V8HI 0 "register_operand" "=v")
diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
index a00d3a3de26..1f6fc03d2ac 100644
--- a/gcc/config/rs6000/mma.md
+++ b/gcc/config/rs6000/mma.md
@@ -540,7 +540,8 @@ (define_insn "mma_<vvi4i4i8>"
MMA_VVI4I4I8))]
"TARGET_MMA"
"<vvi4i4i8> %A0,%x1,%x2,%3,%4,%5"
- [(set_attr "type" "mma")])
+ [(set_attr "type" "mma")
+ (set_attr "prefixed" "yes")])
(define_insn "mma_<avvi4i4i8>"
[(set (match_operand:XO 0 "fpr_reg_operand" "=&d")
@@ -553,7 +554,8 @@ (define_insn "mma_<avvi4i4i8>"
MMA_AVVI4I4I8))]
"TARGET_MMA"
"<avvi4i4i8> %A0,%x2,%x3,%4,%5,%6"
- [(set_attr "type" "mma")])
+ [(set_attr "type" "mma")
+ (set_attr "prefixed" "yes")])
(define_insn "mma_<vvi4i4i2>"
[(set (match_operand:XO 0 "fpr_reg_operand" "=&d")
@@ -565,7 +567,8 @@ (define_insn "mma_<vvi4i4i2>"
MMA_VVI4I4I2))]
"TARGET_MMA"
"<vvi4i4i2> %A0,%x1,%x2,%3,%4,%5"
- [(set_attr "type" "mma")])
+ [(set_attr "type" "mma")
+ (set_attr "prefixed" "yes")])
(define_insn "mma_<avvi4i4i2>"
[(set (match_operand:XO 0 "fpr_reg_operand" "=&d")
@@ -578,7 +581,8 @@ (define_insn "mma_<avvi4i4i2>"
MMA_AVVI4I4I2))]
"TARGET_MMA"
"<avvi4i4i2> %A0,%x2,%x3,%4,%5,%6"
- [(set_attr "type" "mma")])
+ [(set_attr "type" "mma")
+ (set_attr "prefixed" "yes")])
(define_insn "mma_<vvi4i4>"
[(set (match_operand:XO 0 "fpr_reg_operand" "=&d")
@@ -589,7 +593,8 @@ (define_insn "mma_<vvi4i4>"
MMA_VVI4I4))]
"TARGET_MMA"
"<vvi4i4> %A0,%x1,%x2,%3,%4"
- [(set_attr "type" "mma")])
+ [(set_attr "type" "mma")
+ (set_attr "prefixed" "yes")])
(define_insn "mma_<avvi4i4>"
[(set (match_operand:XO 0 "fpr_reg_operand" "=&d")
@@ -601,7 +606,8 @@ (define_insn "mma_<avvi4i4>"
MMA_AVVI4I4))]
"TARGET_MMA"
"<avvi4i4> %A0,%x2,%x3,%4,%5"
- [(set_attr "type" "mma")])
+ [(set_attr "type" "mma")
+ (set_attr "prefixed" "yes")])
(define_insn "mma_<pvi4i2>"
[(set (match_operand:XO 0 "fpr_reg_operand" "=&d")
@@ -612,7 +618,8 @@ (define_insn "mma_<pvi4i2>"
MMA_PVI4I2))]
"TARGET_MMA"
"<pvi4i2> %A0,%x1,%x2,%3,%4"
- [(set_attr "type" "mma")])
+ [(set_attr "type" "mma")
+ (set_attr "prefixed" "yes")])
(define_insn "mma_<apvi4i2>"
[(set (match_operand:XO 0 "fpr_reg_operand" "=&d")
@@ -624,7 +631,8 @@ (define_insn "mma_<apvi4i2>"
MMA_APVI4I2))]
"TARGET_MMA"
"<apvi4i2> %A0,%x2,%x3,%4,%5"
- [(set_attr "type" "mma")])
+ [(set_attr "type" "mma")
+ (set_attr "prefixed" "yes")])
(define_insn "mma_<vvi4i4i4>"
[(set (match_operand:XO 0 "fpr_reg_operand" "=&d")
@@ -636,7 +644,8 @@ (define_insn "mma_<vvi4i4i4>"
MMA_VVI4I4I4))]
"TARGET_MMA"
"<vvi4i4i4> %A0,%x1,%x2,%3,%4,%5"
- [(set_attr "type" "mma")])
+ [(set_attr "type" "mma")
+ (set_attr "prefixed" "yes")])
(define_insn "mma_<avvi4i4i4>"
[(set (match_operand:XO 0 "fpr_reg_operand" "=&d")
@@ -649,4 +658,5 @@ (define_insn "mma_<avvi4i4i4>"
MMA_AVVI4I4I4))]
"TARGET_MMA"
"<avvi4i4i4> %A0,%x2,%x3,%4,%5,%6"
- [(set_attr "type" "mma")])
+ [(set_attr "type" "mma")
+ (set_attr "prefixed" "yes")])
diff --git a/gcc/config/rs6000/pcrel-opt.md b/gcc/config/rs6000/pcrel-opt.md
index 9706a3977c5..f6c9f00e246 100644
--- a/gcc/config/rs6000/pcrel-opt.md
+++ b/gcc/config/rs6000/pcrel-opt.md
@@ -111,7 +111,7 @@ (define_insn "pcrel_opt_ld_addr"
"TARGET_PCREL_OPT
&& reg_or_subregno (operands[0]) != reg_or_subregno (operands[3])"
"ld %0,%a1\n.Lpcrel%2:"
- [(set_attr "prefixed" "yes")
+ [(set_attr "prepend_prefixed_insn" "yes")
(set_attr "type" "load")
(set_attr "loads_external_address" "yes")])
@@ -124,7 +124,7 @@ (define_insn "pcrel_opt_ld_addr_same_reg"
UNSPEC_PCREL_OPT_LD_SAME_REG))]
"TARGET_PCREL_OPT"
"ld %0,%a1\n.Lpcrel%2:"
- [(set_attr "prefixed" "yes")
+ [(set_attr "prepend_prefixed_insn" "yes")
(set_attr "type" "load")
(set_attr "loads_external_address" "yes")])
@@ -299,7 +299,7 @@ (define_insn "*pcrel_opt_st_addr<mode>"
(use (match_operand:PCRELOPT 3 "gpc_reg_operand" "rwa"))]
"TARGET_PCREL_OPT"
"ld %0,%a1\n.Lpcrel%2:"
- [(set_attr "prefixed" "yes")
+ [(set_attr "prepend_prefixed_insn" "yes")
(set_attr "type" "load")
(set_attr "loads_external_address" "yes")])
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 46ddf49d34b..788b283990b 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -26283,7 +26283,8 @@ static bool prepend_p_to_next_insn;
void
rs6000_final_prescan_insn (rtx_insn *insn, rtx [], int)
{
- prepend_p_to_next_insn = (get_attr_prefixed (insn) != PREFIXED_NO);
+ prepend_p_to_next_insn = (get_attr_prepend_prefixed_insn (insn)
+ == PREPEND_PREFIXED_INSN_YES);
return;
}
diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
index c0d7b1aff96..f932ad3988d 100644
--- a/gcc/config/rs6000/rs6000.md
+++ b/gcc/config/rs6000/rs6000.md
@@ -264,13 +264,9 @@ (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 this insn is a prefixed form that needs the macro ASM_OUTPUT_OPCODE
+;; to emit a leading 'p' for the mnemonic.
+(define_attr "prepend_prefixed_insn" "no,yes"
(cond [(ior (match_test "!TARGET_PREFIXED")
(match_test "!NONJUMP_INSN_P (insn)"))
(const_string "no")
@@ -296,6 +292,16 @@ (define_attr "prefixed" "no,yes"
(define_attr "loads_external_address" "no,yes"
(const_string "no"))
+;; Whether an insn is a prefixed insn. A prefixed instruction has a prefix
+;; instruction word that conveys additional information such as a larger
+;; immediate, additional operands, etc., in addition to the normal instruction
+;; word. The default "length" attribute will also be adjusted by default to
+;; be 12 bytes.
+(define_attr "prefixed" "no,yes"
+ (if_then_else (eq_attr "prepend_prefixed_insn" "yes")
+ (const_string "yes")
+ (const_string "no")))
+
;; Return the number of real hardware instructions in a combined insn. If it
;; is 0, just use the length / 4.
(define_attr "num_insns" "" (const_int 0))
@@ -9748,7 +9754,7 @@ (define_insn "*tls_gd_pcrel<bits>"
UNSPEC_TLSGD))]
"HAVE_AS_TLS && TARGET_ELF"
"la %0,%1@got@tlsgd@pcrel"
- [(set_attr "prefixed" "yes")])
+ [(set_attr "prepend_prefixed_insn" "yes")])
(define_insn_and_split "*tls_gd<bits>"
[(set (match_operand:P 0 "gpc_reg_operand" "=b")
@@ -9796,7 +9802,7 @@ (define_insn "*tls_ld_pcrel<bits>"
UNSPEC_TLSLD))]
"HAVE_AS_TLS && TARGET_ELF"
"la %0,%&@got@tlsld@pcrel"
- [(set_attr "prefixed" "yes")])
+ [(set_attr "prepend_prefixed_insn" "yes")])
(define_insn_and_split "*tls_ld<bits>"
[(set (match_operand:P 0 "gpc_reg_operand" "=b")
@@ -9842,7 +9848,7 @@ (define_insn "tls_dtprel_<bits>"
UNSPEC_TLSDTPREL))]
"HAVE_AS_TLS"
"addi %0,%1,%2@dtprel"
- [(set (attr "prefixed")
+ [(set (attr "prepend_prefixed_insn")
(if_then_else (match_test "rs6000_tls_size == 16")
(const_string "no")
(const_string "yes")))])
@@ -9910,7 +9916,7 @@ (define_insn "tls_tprel_<bits>"
UNSPEC_TLSTPREL))]
"HAVE_AS_TLS"
"addi %0,%1,%2@tprel"
- [(set (attr "prefixed")
+ [(set (attr "prepend_prefixed_insn")
(if_then_else (match_test "rs6000_tls_size == 16")
(const_string "no")
(const_string "yes")))])
@@ -9938,7 +9944,7 @@ (define_insn "*tls_got_tprel_pcrel_<bits>"
UNSPEC_TLSGOTTPREL))]
"HAVE_AS_TLS"
"<ptrload> %0,%1@got@tprel@pcrel"
- [(set_attr "prefixed" "yes")])
+ [(set_attr "prepend_prefixed_insn" "yes")])
;; "b" output constraint here and on tls_tls input to support linker tls
;; optimization. The linker may edit the instructions emitted by a
@@ -10253,7 +10259,7 @@ (define_insn "*pcrel_local_addr"
(match_operand:DI 1 "pcrel_local_address"))]
"TARGET_PCREL"
"la %0,%a1"
- [(set_attr "prefixed" "yes")])
+ [(set_attr "prepend_prefixed_insn" "yes")])
;; Load up a PC-relative address to an external symbol. If the symbol and the
;; program are both defined in the main program, the linker will optimize this
@@ -10265,7 +10271,7 @@ (define_insn "*pcrel_extern_addr"
(match_operand:DI 1 "pcrel_external_address"))]
"TARGET_PCREL"
"ld %0,%a1"
- [(set_attr "prefixed" "yes")
+ [(set_attr "prepend_prefixed_insn" "yes")
(set_attr "type" "load")
(set_attr "loads_external_address" "yes")])
@@ -11722,7 +11728,7 @@ (define_insn "stack_protect_setdi"
;; Back to back prefixed memory instructions take 20 bytes (8 bytes for each
;; prefixed instruction + 4 bytes for the possible NOP). Add in 4 bytes for
;; the LI 0 at the end.
- (set_attr "prefixed" "no")
+ (set_attr "prepend_prefixed_insn" "no")
(set_attr "num_insns" "3")
(set (attr "length")
(cond [(and (match_operand 0 "prefixed_memory")
@@ -11804,7 +11810,7 @@ (define_insn "stack_protect_testdi"
;; Back to back prefixed memory instructions take 20 bytes (8 bytes for each
;; prefixed instruction + 4 bytes for the possible NOP). Add in either 4 or
;; 8 bytes to do the test.
- [(set_attr "prefixed" "no")
+ [(set_attr "prepend_prefixed_insn" "no")
(set_attr "num_insns" "4,5")
(set (attr "length")
(cond [(and (match_operand 1 "prefixed_memory")
diff --git a/gcc/config/rs6000/sync.md b/gcc/config/rs6000/sync.md
index 40629dd9eec..3f902ce981a 100644
--- a/gcc/config/rs6000/sync.md
+++ b/gcc/config/rs6000/sync.md
@@ -132,9 +132,10 @@ (define_insn "load_quadpti"
"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")))])
+ (set (attr "prepend_prefixed_insn")
+ (if_then_else (match_test "TARGET_PREFIXED")
+ (const_string "yes")
+ (const_string "no")))])
;; Pattern load_quadpti will always use plq for atomic TImode if
;; TARGET_PREFIXED. It has the correct doubleword ordering on either LE
@@ -207,9 +208,10 @@ (define_insn "store_quadpti"
"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")))])
+ (set (attr "prepend_prefixed_insn")
+ (if_then_else (match_test "TARGET_PREFIXED")
+ (const_string "yes")
+ (const_string "no")))])
;; Pattern store_quadpti will always use pstq if TARGET_PREFIXED,
;; so the doubleword swap is never needed in that case.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, rs6000 V2] Update "prefix" attribute for Power10 [PR99133]
2021-03-17 20:49 [PATCH, rs6000 V2] Update "prefix" attribute for Power10 [PR99133] Pat Haugen
@ 2021-03-18 16:33 ` will schmidt
2021-03-18 20:01 ` Pat Haugen
2021-03-23 16:50 ` Segher Boessenkool
1 sibling, 1 reply; 4+ messages in thread
From: will schmidt @ 2021-03-18 16:33 UTC (permalink / raw)
To: Pat Haugen, GCC Patches
Cc: Peter Bergner, Bill Schmidt, David Edelsohn, Segher Boessenkool
On Wed, 2021-03-17 at 15:49 -0500, Pat Haugen via Gcc-patches wrote:
> Update prefixed attribute for Power10.
>
> This patch creates a new attribute, prepend_prefixed_insn, which is
> used to mark
> those instructions that are prefixed and need to have a 'p' prepended
> to their
> mnemonic at asm emit time. The existing "prefix" attribute is now
> used to mark
> all instructions that are prefixed form.
>
> Bootstrap/regtest on powerpc64le (Power10) and powerpc64 (Power8
> 32/64) with no
> new regressions. Ok for trunk?
>
> -Pat
>
>
> 2021-03-17 Pat Haugen <pthaugen@linux.ibm.com>
>
> gcc/
> PR target/99133
> * config/rs6000/altivec.md (xxspltiw_v4si, xxspltiw_v4sf_inst,
> xxspltidp_v2df_inst, xxsplti32dx_v4si_inst,
> xxsplti32dx_v4sf_inst,
> xxblend_<mode>, xxpermx_inst, xxeval): Mark prefixed.
> * config/rs6000/mma.md (mma_<vvi4i4i8>, mma_<avvi4i4i8>,
> mma_<vvi4i4i2>, mma_<avvi4i4i2>, mma_<vvi4i4>, mma_<avvi4i4>,
> mma_<pvi4i2>, mma_<apvi4i2>, mma_<vvi4i4i4>, mma_<avvi4i4i4>):
> Likewise.
> * config/rs6000/pcrel-opt.md: Adjust attribute name.
> * config/rs6000/rs6000.c (rs6000_final_prescan_insn): Adjust
> test.
> * config/rs6000/rs6000.md (define_attr
> "prepend_prefixed_insn"): New.
> (define_attr "prefixed"): Update initializer.
> (*tls_gd_pcrel<bits>, *tls_ld_pcrel<bits>, tls_dtprel_<bits>,
> tls_tprel_<bits>, *tls_got_tprel_pcrel_<bits>,
> *pcrel_local_addr,
> *pcrel_extern_addr, stack_protect_setdi, stack_protect_testdi):
> Adjust attribute name.
> * config/rs6000/sync.md (load_quadpti, store_quadpti):
> Likewise.
>
>
Changelog matches patch contents. (ok!) :-)
Per this change:
+;; Whether an insn is a prefixed insn. A prefixed instruction has a prefix
+;; instruction word that conveys additional information such as a larger
+;; immediate, additional operands, etc., in addition to the normal instruction
+;; word. The default "length" attribute will also be adjusted by default to
+;; be 12 bytes.
+(define_attr "prefixed" "no,yes"
+ (if_then_else (eq_attr "prepend_prefixed_insn" "yes")
+ (const_string "yes")
+ (const_string "no")))
.. it looks like at least most of the users of the "prefixed" attribute have
been switched over to use "prepend_prefixed_insn" instead. Are there still
users of the "prefixed" attribute remaining ? I'm guessing so, given context,
but can't tell for certain.
(Just a question, not a specific request for a change)
lgtm
thanks
-Will
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, rs6000 V2] Update "prefix" attribute for Power10 [PR99133]
2021-03-18 16:33 ` will schmidt
@ 2021-03-18 20:01 ` Pat Haugen
0 siblings, 0 replies; 4+ messages in thread
From: Pat Haugen @ 2021-03-18 20:01 UTC (permalink / raw)
To: will schmidt, GCC Patches
Cc: Peter Bergner, Bill Schmidt, David Edelsohn, Segher Boessenkool
On 3/18/21 11:33 AM, will schmidt wrote:
> Per this change:
>
> +;; Whether an insn is a prefixed insn. A prefixed instruction has a prefix
> +;; instruction word that conveys additional information such as a larger
> +;; immediate, additional operands, etc., in addition to the normal instruction
> +;; word. The default "length" attribute will also be adjusted by default to
> +;; be 12 bytes.
> +(define_attr "prefixed" "no,yes"
> + (if_then_else (eq_attr "prepend_prefixed_insn" "yes")
> + (const_string "yes")
> + (const_string "no")))
>
>
> .. it looks like at least most of the users of the "prefixed" attribute have
> been switched over to use "prepend_prefixed_insn" instead. Are there still
> users of the "prefixed" attribute remaining ? I'm guessing so, given context,
> but can't tell for certain.
>
> (Just a question, not a specific request for a change)
Yes, there are still a couple uses of get_attr_prefixed() in rs6000.c, plus the Power10 scheduling description makes use of it.
-Pat
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, rs6000 V2] Update "prefix" attribute for Power10 [PR99133]
2021-03-17 20:49 [PATCH, rs6000 V2] Update "prefix" attribute for Power10 [PR99133] Pat Haugen
2021-03-18 16:33 ` will schmidt
@ 2021-03-23 16:50 ` Segher Boessenkool
1 sibling, 0 replies; 4+ messages in thread
From: Segher Boessenkool @ 2021-03-23 16:50 UTC (permalink / raw)
To: Pat Haugen; +Cc: GCC Patches, David Edelsohn, Peter Bergner, Bill Schmidt
Hi!
On Wed, Mar 17, 2021 at 03:49:09PM -0500, Pat Haugen wrote:
> This patch creates a new attribute, prepend_prefixed_insn, which is used to mark
> those instructions that are prefixed and need to have a 'p' prepended to their
> mnemonic at asm emit time. The existing "prefix" attribute is now used to mark
> all instructions that are prefixed form.
But it doesn't. The "prepend_prefixed_insn" attribute forces the
"prefixed" attribute to be on, whether the insn is prefixed or not.
Please change it so that "prefixed" just means whether the instruction
is prefixed, and a new attribute "maybe_prefixed" that marks the insns
that potentially are prefixed. Similar to how "var_shift" and
"maybe_var_shift" work. The default value of "prefixed" can set the
attribute to either "yes" if "maybe_prefixed" is set, and the insn
should get a "p" mnemonic.
Btw, attributes describe some property of insns. They do not say what
any particular part of the compiler should do with the insns.
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -26283,7 +26283,8 @@ static bool prepend_p_to_next_insn;
> void
> rs6000_final_prescan_insn (rtx_insn *insn, rtx [], int)
> {
> - prepend_p_to_next_insn = (get_attr_prefixed (insn) != PREFIXED_NO);
> + prepend_p_to_next_insn = (get_attr_prepend_prefixed_insn (insn)
> + == PREPEND_PREFIXED_INSN_YES);
> return;
> }
But the new attribute here is set if the insn *can* be prefixed, not if
it *is*.
> --- a/gcc/config/rs6000/sync.md
> +++ b/gcc/config/rs6000/sync.md
> @@ -132,9 +132,10 @@ (define_insn "load_quadpti"
> "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")))])
> + (set (attr "prepend_prefixed_insn")
> + (if_then_else (match_test "TARGET_PREFIXED")
> + (const_string "yes")
> + (const_string "no")))])
(bad whitespace)
I don't think this is correct. We need to force the use of "plq" here
(for LE only actually, that should be fixed some day). Maybe we should
have a separate instruction pattern for it, even, since its semantics
differ very significantly :-)
Segher
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-23 16:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-17 20:49 [PATCH, rs6000 V2] Update "prefix" attribute for Power10 [PR99133] Pat Haugen
2021-03-18 16:33 ` will schmidt
2021-03-18 20:01 ` Pat Haugen
2021-03-23 16:50 ` 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).