diff options
author | Jeff Law <jlaw@ventanamicro.com> | 2025-03-17 17:29:42 -0600 |
---|---|---|
committer | Jeff Law <jlaw@ventanamicro.com> | 2025-03-17 17:30:30 -0600 |
commit | d9a8ec7fe0cbc04e28e650f079952bf529ae612e (patch) | |
tree | adc161c4510a886e9723cc53c43e94178e9f1bd1 | |
parent | 456f5ef81f0c6de630a60c26341082fffd48f241 (diff) | |
download | gcc-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.md | 20 | ||||
-rw-r--r-- | gcc/testsuite/gcc.target/riscv/redundant-andi.c | 41 | ||||
-rw-r--r-- | gcc/testsuite/gcc.target/riscv/redundant-ori.c | 18 |
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" } } */ + |