RISC-V: Add Types to Missing Bitmanip Instructions:

Message ID 20230821163852.988058-1-ewlu@rivosinc.com
State Unresolved
Headers
Series RISC-V: Add Types to Missing Bitmanip Instructions: |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Edwin Lu Aug. 21, 2023, 4:37 p.m. UTC
  Related Discussion:
https://inbox.sourceware.org/gcc-patches/12fb5088-3f28-0a69-de1e-f387371a5eb2@gmail.com/

This patch updates the bitmanip instructions to ensure that no insn is left
without a type attribute. Updates a total of 8 insns to have type "bitmanip"

Tested for regressions using rv32/64 multilib with newlib/linux. 

gcc/Changelog:

	* config/riscv/bitmanip.md: Added bitmanip type to insns
    that are missing types

Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
---
 gcc/config/riscv/bitmanip.md | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)
  

Comments

Jeff Law Aug. 21, 2023, 9:24 p.m. UTC | #1
On 8/21/23 10:37, Edwin Lu wrote:
> This patch updates the bitmanip instructions to ensure that no insn is left
> without a type attribute. Updates a total of 8 insns to have type "bitmanip"
> 
> Tested for regressions using rv32/64 multilib with newlib/linux.
> 
> gcc/Changelog:
> 
> 	* config/riscv/bitmanip.md: Added bitmanip type to insns
>      that are missing types
Thanks.  I pushed this to the trunk.

Just an FYI.  We have a bit of leeway on the types for these 
define_insn_and_split patterns.  So while I think that 'branch' is a 
better choice for the last one in this patch, I don't think it's going 
to matter in practice.

Essentially these define_insn_and_split patterns which are always split 
after reload, the type will only be used for the first scheduling pass. 
By the time we run the second scheduling pass the pattern should have 
been split into its component insns.  Meaning that these types won't get 
used for the final schedule.

But it's still useful to get a type on them, mostly because it'll allow 
us to turn on that assert I mentioned last week.


jeff
  

Patch

diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index c42e7b890db..0c99152ffc8 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -103,7 +103,8 @@  (define_insn_and_split "*andi_add.uw"
 			       (match_dup 4)))]
 {
 	operands[3] = GEN_INT (INTVAL (operands[3]) >> INTVAL (operands[2]));
-})
+}
+[(set_attr "type" "bitmanip")])
 
 (define_insn "*shNadduw"
   [(set (match_operand:DI 0 "register_operand" "=r")
@@ -533,7 +534,9 @@  (define_insn_and_split "*minmax"
   "&& reload_completed"
   [(set (match_dup 3) (sign_extend:DI (match_dup 1)))
    (set (match_dup 4) (match_dup 2))
-   (set (match_dup 0) (<minmax_optab>:DI (match_dup 3) (match_dup 4)))])
+   (set (match_dup 0) (<minmax_optab>:DI (match_dup 3) (match_dup 4)))]
+  ""
+  [(set_attr "type" "bitmanip")])
 
 ;; ZBS extension.
 
@@ -628,7 +631,8 @@  (define_insn_and_split "*bclri<mode>_nottwobits"
 
 	operands[3] = GEN_INT (~bits | topbit);
 	operands[4] = GEN_INT (~topbit);
-})
+}
+[(set_attr "type" "bitmanip")])
 
 ;; In case of a paradoxical subreg, the sign bit and the high bits are
 ;; not allowed to be changed
@@ -648,7 +652,8 @@  (define_insn_and_split "*bclridisi_nottwobits"
 
 	operands[3] = GEN_INT (~bits | topbit);
 	operands[4] = GEN_INT (~topbit);
-})
+}
+[(set_attr "type" "bitmanip")])
 
 (define_insn "*binv<mode>"
   [(set (match_operand:X 0 "register_operand" "=r")
@@ -743,7 +748,8 @@  (define_insn_and_split "*<or_optab>i<mode>_extrabit"
 
 	operands[3] = GEN_INT (bits &~ topbit);
 	operands[4] = GEN_INT (topbit);
-})
+}
+[(set_attr "type" "bitmanip")])
 
 ;; Same to use blcri + andi and blcri + bclri
 (define_insn_and_split "*andi<mode>_extrabit"
@@ -761,7 +767,8 @@  (define_insn_and_split "*andi<mode>_extrabit"
 
 	operands[3] = GEN_INT (bits | topbit);
 	operands[4] = GEN_INT (~topbit);
-})
+}
+[(set_attr "type" "bitmanip")])
 
 ;; IF_THEN_ELSE: test for 2 bits of opposite polarity
 (define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit"
@@ -803,7 +810,8 @@  (define_insn_and_split "*branch<X:mode>_mask_twobits_equals_singlebit"
 
    operands[8] = GEN_INT (setbit);
    operands[9] = GEN_INT (clearbit);
-})
+}
+[(set_attr "type" "bitmanip")])
 
 ;; IF_THEN_ELSE: test for (a & (1 << BIT_NO))
 (define_insn_and_split "*branch<X:mode>_bext"
@@ -826,7 +834,9 @@  (define_insn_and_split "*branch<X:mode>_bext"
 					(zero_extend:X (match_dup 3))))
    (set (pc) (if_then_else (match_op_dup 1 [(match_dup 4) (const_int 0)])
 			   (label_ref (match_dup 0))
-			   (pc)))])
+			   (pc)))]
+   ""
+  [(set_attr "type" "bitmanip")])
 
 ;; ZBKC or ZBC extension
 (define_insn "riscv_clmul_<mode>"