aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff Law <jlaw@ventanamicro.com>2025-03-17 17:29:42 -0600
committerJeff Law <jlaw@ventanamicro.com>2025-03-17 17:30:30 -0600
commitd9a8ec7fe0cbc04e28e650f079952bf529ae612e (patch)
treeadc161c4510a886e9723cc53c43e94178e9f1bd1
parent456f5ef81f0c6de630a60c26341082fffd48f241 (diff)
downloadgcc-d9a8ec7fe0cbc04e28e650f079952bf529ae612e.zip
gcc-d9a8ec7fe0cbc04e28e650f079952bf529ae612e.tar.gz
gcc-d9a8ec7fe0cbc04e28e650f079952bf529ae612e.tar.bz2
[RISC-V] Fix unreported code quality regression with single bit manipulations
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>extrabit): Likewise. gcc/testsuite/ * gcc.target/riscv/redundant-andi.c: New test. * gcc.target/riscv/redundant-ori.c: Likewise
-rw-r--r--gcc/config/riscv/bitmanip.md20
-rw-r--r--gcc/testsuite/gcc.target/riscv/redundant-andi.c41
-rw-r--r--gcc/testsuite/gcc.target/riscv/redundant-ori.c18
3 files changed, 69 insertions, 10 deletions
diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index 684e5d2..b29c127 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -1017,17 +1017,17 @@
[(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 @@
[(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 0000000..8945faf
--- /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 0000000..e863343
--- /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" } } */
+