[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

diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index 684e5d2ae8b6308cdf3d295f3a696d369a3fbec4..b29c127bcb841e27c3cc660d85a75cbc4b45e0c2 100644 (file)
--- 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 (file)
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 (file)
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" } } */
+