As is outlined in the PR, we have a few define_insn_and_split patterns
which optimize away explicit masking of shift/bit positions when the
masking matches what the hardware's behavior.
A small number of those define_insn_and_split patterns generate a single
instruction. It's fairly elegant in that we were essentially just
rewriting the RTL to match an existing pattern.
In one case we'd do the rewriting and later turn a 32bit shift into a
bset. That's not safe because the masking of a 32bit shift uses 0x1f
while masking on bset uses 0x3f on rv64. The net was incorrect code as
seen in the BZ entry.
The fix is pretty simple. There's no real reason we need to use a
define_insn_and_split. It was just convenient. Instead we can use a
simple define_insn. That avoids a change in the masking behavior for
the shift count/bit position and the masking stays in the RTL.
I quickly scanned the entire port and didn't see any additional
define_insn_and_splits that obviously generated a single instruction
outside the shift/rotate space, though in the vector space that's
nontrivial to ascertain.
This was been run through my tester for the cross configurations, but
not the native bootstrap/regression test (yet).
I'll obviously wait for upstream pre-commit testing before moving forward.
jeff
PR target/119971
gcc/
* config/riscv/bitmanip.md (rotation with masked count): Rewrite
as define_insn patterns. Fix formatting.
* config/riscv/riscv.md (shift with masked count): Similarly.
gcc/testuite/
* gcc.target/riscv/pr119971.c: New test.
diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index d0919ece31f..20d03dc8792 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -423,39 +423,40 @@ (define_insn "rotlsi3_sext"
"rolw\t%0,%1,%2"
[(set_attr "type" "bitmanip")])
-(define_insn_and_split "*<bitmanip_optab><GPR:mode>3_mask"
- [(set (match_operand:GPR 0 "register_operand" "= r")
- (bitmanip_rotate:GPR
- (match_operand:GPR 1 "register_operand" " r")
- (match_operator 4 "subreg_lowpart_operator"
- [(and:GPR2
- (match_operand:GPR2 2 "register_operand" "r")
- (match_operand 3 "<GPR:shiftm1>" "<GPR:shiftm1p>"))])))]
+(define_insn "*<bitmanip_optab><mode>3_mask"
+ [(set (match_operand:X 0 "register_operand" "=r")
+ (bitmanip_rotate:X
+ (match_operand:X 1 "register_operand" "r")
+ (match_operator 4 "subreg_lowpart_operator"
+ [(and:X (match_operand:X 2 "register_operand" "r")
+ (match_operand 3 "<X:shiftm1>" "<X:shiftm1p>"))])))]
"TARGET_ZBB || TARGET_ZBKB"
- "#"
- "&& 1"
- [(set (match_dup 0)
- (bitmanip_rotate:GPR (match_dup 1)
- (match_dup 2)))]
- "operands[2] = gen_lowpart (QImode, operands[2]);"
+ "<bitmanip_insn>\t%0,%1,%2"
[(set_attr "type" "bitmanip")
- (set_attr "mode" "<GPR:MODE>")])
+ (set_attr "mode" "<X:MODE>")])
-(define_insn_and_split "*<bitmanip_optab>si3_sext_mask"
- [(set (match_operand:DI 0 "register_operand" "= r")
- (sign_extend:DI (bitmanip_rotate:SI
- (match_operand:SI 1 "register_operand" " r")
- (match_operator 4 "subreg_lowpart_operator"
- [(and:GPR
- (match_operand:GPR 2 "register_operand" "r")
- (match_operand 3 "const_si_mask_operand"))]))))]
+(define_insn "*<bitmanip_optab>3_mask_si"
+ [(set (match_operand:SI 0 "register_operand" "=r")
+ (bitmanip_rotate:SI
+ (match_operand:SI 1 "register_operand" "r")
+ (match_operator 3 "subreg_lowpart_operator"
+ [(and:X (match_operand:SI 2 "register_operand" "r")
+ (const_int 31))])))]
"TARGET_64BIT && (TARGET_ZBB || TARGET_ZBKB)"
- "#"
- "&& 1"
- [(set (match_dup 0)
- (sign_extend:DI (bitmanip_rotate:SI (match_dup 1)
- (match_dup 2))))]
- "operands[2] = gen_lowpart (QImode, operands[2]);"
+ "<bitmanip_insn>w\t%0,%1,%2"
+ [(set_attr "type" "bitmanip")
+ (set_attr "mode" "SI")])
+
+(define_insn "*<bitmanip_optab>si3_sext_mask"
+ [(set (match_operand:DI 0 "register_operand" "=r")
+ (sign_extend:DI
+ (bitmanip_rotate:SI
+ (match_operand:SI 1 "register_operand" "r")
+ (match_operator 3 "subreg_lowpart_operator"
+ [(and:X (match_operand:GPR 2 "register_operand" "r")
+ (const_int 31))]))))]
+ "TARGET_64BIT && (TARGET_ZBB || TARGET_ZBKB)"
+ "<bitmanip_insn>w\t%0,%1,%2"
[(set_attr "type" "bitmanip")
(set_attr "mode" "DI")])
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index c34eadb01c9..15c89ff4e3d 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -2937,7 +2937,7 @@ (define_insn "<optab>di3"
[(set_attr "type" "shift")
(set_attr "mode" "DI")])
-(define_insn_and_split "*<optab><GPR:mode>3_mask_1"
+(define_insn "*<optab><GPR:mode>3_mask_1"
[(set (match_operand:GPR 0 "register_operand" "= r")
(any_shift:GPR
(match_operand:GPR 1 "register_operand" " r")
@@ -2946,12 +2946,7 @@ (define_insn_and_split "*<optab><GPR:mode>3_mask_1"
(match_operand:GPR2 2 "register_operand" "r")
(match_operand 3 "<GPR:shiftm1>"))])))]
""
- "#"
- "&& 1"
- [(set (match_dup 0)
- (any_shift:GPR (match_dup 1)
- (match_dup 2)))]
- "operands[2] = gen_lowpart (QImode, operands[2]);"
+ "<insn>\t%0,%1,%2"
[(set_attr "type" "shift")
(set_attr "mode" "<GPR:MODE>")])
@@ -2970,7 +2965,7 @@ (define_insn "<optab>si3_extend"
[(set_attr "type" "shift")
(set_attr "mode" "SI")])
-(define_insn_and_split "*<optab>si3_extend_mask"
+(define_insn "*<optab>si3_extend_mask"
[(set (match_operand:DI 0 "register_operand" "= r")
(sign_extend:DI
(any_shift:SI
@@ -2980,13 +2975,7 @@ (define_insn_and_split "*<optab>si3_extend_mask"
(match_operand:GPR 2 "register_operand" " r")
(match_operand 3 "const_si_mask_operand"))]))))]
"TARGET_64BIT"
- "#"
- "&& 1"
- [(set (match_dup 0)
- (sign_extend:DI
- (any_shift:SI (match_dup 1)
- (match_dup 2))))]
- "operands[2] = gen_lowpart (QImode, operands[2]);"
+ "<insn>w\t%0,%1,%2"
[(set_attr "type" "shift")
(set_attr "mode" "SI")])
diff --git a/gcc/testsuite/gcc.target/riscv/pr119971.c
b/gcc/testsuite/gcc.target/riscv/pr119971.c
new file mode 100644
index 00000000000..c3f23b05ec3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr119971.c
@@ -0,0 +1,24 @@
+/* { dg-do compile { target rv64 } } */
+/* { dg-options "-march=rv64gcb -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-g" "-Oz" "-Os" } } */
+
+__attribute__ ((noipa)) unsigned
+foo (unsigned b, unsigned e, unsigned i)
+{
+ e >>= b;
+ i >>= e & 31;
+ return i & 1;
+}
+
+int main()
+{
+ if (foo (0x18, 0xfe000000, 0x40000000) != 1)
+ __builtin_abort ();
+ return 0;
+}
+
+/* { dg-final { scan-assembler-times "andi\t" 1 } } */
+/* { dg-final { scan-assembler-times "srlw\t" 2 } } */
+/* { dg-final { scan-assembler-not "bext\t" } } */
+
+
