On 7/8/2026 9:07 AM, Milan Tripkovic wrote:
This patch fixes PR123905, where the compiler generates
redundant sign extensions and bit masks on RISC-V when
using __builtin_clz, __builtin_ctz, and __builtin_popcount.
To solve this, the patch introduces the following changes
based on Jeff's suggestion:
ext-dce.cc: Updated ext_dce_try_optimize_extension to prevent
the conversion of a SIGN_EXTEND to a subreg if the source's sign bit
is already known to be zero.
rtlanal.cc: Improve nonzero_bits1 and num_sign_bit_copies1 so that
they recognize that the results of CLZ, CTZ, and POPCOUNT are
bounded by floor_log2 (bitwidth) + 1 bits, meaning that all higher
bits are known to be zero.
ASM Before:
clzw a0,a0
andn t0,a0,a1
sext.w a0,t0
ret
ASM After:
clzw a0,a0
andn a0,a0,a1
ret
The redundant sext.w is eliminated because the compiler now knows that
the result already has a zero sign bit.
Additionally, this patch includes a new test case
gcc.target/riscv/pr123905.c
which covers several scenarios: AND, ANDN, and combinations like clz & ctz
and clz & ctz & popcount.
2026-08-06 Milan Tripkovic <[email protected]>
gcc/ChangeLog:
* ext-dce.cc (ext_dce_try_optimize_extension): prevent conversion.
* rtlanal.cc (nonzero_bits1):fixed mask.
(num_sign_bit_copies1): Add CLZ/CTZ/POPCOUNT handling.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/pr123905.c: New test case.
diff --git a/gcc/ext-dce.cc b/gcc/ext-dce.cc
index 9e4b0a429b..e89b6e1778 100644
--- a/gcc/ext-dce.cc
+++ b/gcc/ext-dce.cc
@@ -538,6 +538,44 @@ ext_dce_try_optimize_extension (rtx_insn *insn,
rtx set)
if (!(REG_P (inner) || (SUBREG_P (inner) && REG_P (SUBREG_REG
(inner)))))
return;
+/* Keep SIGN_EXTEND when DEF_SRC sign bit is known zero. */
+ if (GET_CODE (src) == SIGN_EXTEND)
+ {
+ rtx def_reg = inner;
+ scalar_int_mode inner_mode;
+
+ if (SUBREG_P (def_reg))
+ def_reg = SUBREG_REG (def_reg);
[ ... ]
Presumably this complexity is the need to walk back in the IL to find
the definition (a ctz/clz/popcount) so we can use that to compute the
sign bit copies?
+
+ if (regno >= FIRST_PSEUDO_REGISTER && DF_REG_DEF_COUNT (regno)
== 1)
Good. We don't have to worry about merging when you get a DEF count of 1.
+ {
+ df_ref def = DF_REG_DEF_CHAIN (regno);
+ if (def)
+ {
+ rtx_insn *def_insn = DF_REF_INSN (def);
+ rtx def_set = def_insn ? single_set (def_insn) : NULL_RTX;
+
+ if (def_set)
+ {
+ rtx def_src = SET_SRC (def_set);
+ unsigned HOST_WIDE_INT nz
+ = nonzero_bits (def_src, inner_mode);
+ unsigned HOST_WIDE_INT sign_mask
+ = HOST_WIDE_INT_1U << (GET_MODE_PRECISION (inner_mode)-1);
+
+ if ((nz & sign_mask) == 0)
+ return;
+ }
What I was thinking was testing the sign bit copies. Given the outer
and inner modes, something like:
num_sign_bit_copies (ctz expression) > GET_MODE_PRECISION (outer) -
GET_MODE_PRECISION (inner)
I think both your approach and mine probably get us where we want to
be. I think mine's marginally better in that it'd apply any time we
have a trivially redundant sign extension which I've seen cause problems
in other cases when ext-dce removes the extension leaving the resulting
IL harder for combine.c to optimize due to the SUBREGs.
That would tell us when the sign extension is trivially redundant (the
value is already extended). Those should be eliminable by combine
pretty easily. You may want to refactor this into a new function.
Right now it'd just handle the case we're talking about, but I could
easily see it expanding in the future if there are other cases where we
want to leave an extension in the IL.
For the rtlanal.cc changes, I think the indention for nonzero_bits1 is
wrong. The open curley should be indented two spaces from the case
statement. If you look at the IF_THEN_ELSE case a dozen or so lines
after your new code for FFS, POPCOUNT, CLZ and CTZ you'd see proper
indentation. The comment for your new code in sign_bit_copies should
indent inside the open curley. You probably want a vertical whitespace
to separate the CLZ/CTZ/POPCOUNT case from the REG case immediately
afterward.
Anyway, I think you're on the right track here.
jeff
