I was reviewing some code recently and spotted an oddity. In a few
places we were emitting andi dst,src,-1 and in others [x]ori dst,src,0.
Those are obviously nops and we should get rid of them.
Most of these are coming from a split part of a couple
define_insn_and_split patterns added back in late 2022, so this is an
unreported 13, 14 & 15 code quality regression (verified on godbolt,
https://godbolt.org/z/EPszox5Kd). Essentially the split part is
matching over-aggressively and splitting what should be a trivial
bitmanip insn such as bset, bclr or binv into a nop logical with a bit
twiddle.
Since the split portions trigger post-reload nothing comes along to
remove the nop logical operations.
The fix is trivial. Just refine the condition. I considered refining
the operand predicates too. Both are valid approaches. I noticed the
formatting was goofy, so fixed that while I was in there.
I'm aware of one other similar case, but I haven't concluded if it's a
regression or not.
Tested in my tester. Waiting for pre-commit CI to do its thing.
Jeff
gcc/
* config/riscv/bitmanip.md (*<or_optab>i<mode>_extrabit): Reject cases
where we only need to twiddle one bit. Fix formatting.
*(andi<mode>e_xtrabit): Likewise.
gcc/testsuite/
* gcc.target/riscv/redundant-andi.c: New test.
* gcc.target/riscv/redundant-ori.c: Likewise
diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index 684e5d2ae8b..b29c127bcb8 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -1017,17 +1017,17 @@ (define_insn_and_split "*<or_optab>i<mode>_extrabit"
[(set (match_operand:X 0 "register_operand" "=r")
(any_or:X (match_operand:X 1 "register_operand" "r")
(match_operand:X 2 "uimm_extra_bit_or_twobits" "i")))]
- "TARGET_ZBS"
+ "TARGET_ZBS && !single_bit_mask_operand (operands[2], VOIDmode)"
"#"
"&& reload_completed"
[(set (match_dup 0) (<or_optab>:X (match_dup 1) (match_dup 3)))
(set (match_dup 0) (<or_optab>:X (match_dup 0) (match_dup 4)))]
{
- unsigned HOST_WIDE_INT bits = UINTVAL (operands[2]);
- unsigned HOST_WIDE_INT topbit = HOST_WIDE_INT_1U << floor_log2 (bits);
+ unsigned HOST_WIDE_INT bits = UINTVAL (operands[2]);
+ unsigned HOST_WIDE_INT topbit = HOST_WIDE_INT_1U << floor_log2 (bits);
- operands[3] = GEN_INT (bits &~ topbit);
- operands[4] = GEN_INT (topbit);
+ operands[3] = GEN_INT (bits &~ topbit);
+ operands[4] = GEN_INT (topbit);
}
[(set_attr "type" "bitmanip")])
@@ -1036,17 +1036,17 @@ (define_insn_and_split "*andi<mode>_extrabit"
[(set (match_operand:X 0 "register_operand" "=r")
(and:X (match_operand:X 1 "register_operand" "r")
(match_operand:X 2 "not_uimm_extra_bit_or_nottwobits" "i")))]
- "TARGET_ZBS"
+ "TARGET_ZBS && !not_single_bit_mask_operand (operands[2], VOIDmode)"
"#"
"&& reload_completed"
[(set (match_dup 0) (and:X (match_dup 1) (match_dup 3)))
(set (match_dup 0) (and:X (match_dup 0) (match_dup 4)))]
{
- unsigned HOST_WIDE_INT bits = UINTVAL (operands[2]);
- unsigned HOST_WIDE_INT topbit = HOST_WIDE_INT_1U << floor_log2 (~bits);
+ unsigned HOST_WIDE_INT bits = UINTVAL (operands[2]);
+ unsigned HOST_WIDE_INT topbit = HOST_WIDE_INT_1U << floor_log2 (~bits);
- operands[3] = GEN_INT (bits | topbit);
- operands[4] = GEN_INT (~topbit);
+ operands[3] = GEN_INT (bits | topbit);
+ operands[4] = GEN_INT (~topbit);
}
[(set_attr "type" "bitmanip")])
diff --git a/gcc/testsuite/gcc.target/riscv/redundant-andi.c
b/gcc/testsuite/gcc.target/riscv/redundant-andi.c
new file mode 100644
index 00000000000..8945faf8888
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/redundant-andi.c
@@ -0,0 +1,41 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcb -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" } } */
+
+
+typedef struct sv SV;
+
+typedef struct magic MAGIC;
+typedef short I16;
+typedef unsigned short U16;
+typedef int I32;
+typedef unsigned int U32;
+struct sv
+{
+ U32 sv_refcnt;
+ U32 sv_flags;
+};
+struct magic
+{
+ U16 mg_private;
+};
+extern SV **PL_psig_ptr;
+int
+Perl_magic_setsig (SV *sv, MAGIC *mg, const char *s)
+{
+ I32 i;
+ i = (I16) mg->mg_private;
+ if (sv)
+ {
+ PL_psig_ptr[i] = (++((sv)->sv_refcnt), ((SV *) ((void *) (sv))));
+ ((sv)->sv_flags &= ~0x00080000);
+ }
+ else
+ {
+ PL_psig_ptr[i] = ((void *) 0);
+ }
+ return 0;
+}
+
+/* { dg-final { scan-assembler-not "andi\t" } } */
+
diff --git a/gcc/testsuite/gcc.target/riscv/redundant-ori.c
b/gcc/testsuite/gcc.target/riscv/redundant-ori.c
new file mode 100644
index 00000000000..e863343b561
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/redundant-ori.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcb -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" } } */
+
+
+struct sv XS_constant__make_const_svz_2_0;
+struct sv {
+ int sv_flags;
+} XS_constant__make_const() {
+ struct sv *sv = &XS_constant__make_const_svz_2_0;
+ sv->sv_flags |= 65536;
+ if (XS_constant__make_const_svz_2_0.sv_flags & 5)
+ for (;;)
+ ;
+}
+
+/* { dg-final { scan-assembler-not "ori\t" } } */
+
