> Note that canonicalization is driven more by the SSA_NAME_VERSION and IL
> structure than types and getting things in the right form to expose this
> case may be hard.
>
> One approach I've taken through the years is to sketch out such a
> pattern and have it abort() when it matches. Then I run a large
> codebase through that compiler to see if I can get it to trigger that
> abort. Much like basic pattern development, I often start with a
> simplistic, over-matching pattern and refine. That makes it less likely
> that a logical big preventing a match makes me conclude the pattern is
> unnecessary/worthless. Assuming that process finds a real trigger, I
> then reduce it and include going forward.
>
> It's not critical for this patch IMHO, but perhaps an avenue to explore
> in the future. Based on Andrea's comments, I suspect we're in agreement
> on that.
As I mentioned, usually I just try different combinations of the same
testcase to see if it behaves differently, but your suggestion makes a
lot of sense too, though I haven't tried it for this version of the
patch.
> Don't the tests in the IF clause result in type <= ltype0 <= ltype1, yet
> in the WITH clause we're assigning largertype to ltype0. Something
> seems wrong with that.
Yes, and now that you mention it largertype does seem redundant. I
removed it in this version and used @4 (associated with ltype1) instead
of @1.
> Your scan-assembler-times tests are a bit inconsistent in that they
> don't anchor with a tab before or after the opcode with any
> consistency. We're not terribly good about this in the testsuite, so
> it's not a major problem, though we should try to be consistent within
> any given test if we can.
I added the tab at first to differentiate between andi and and, so now I
have it on all scan-assembler-times tests for consistency.
Also, I tweaked the values in the testcase b a bit because for some
reason the assembly generated for rv32 wasn't branchless. It is
branchless now.
> Overall I think you're on the right track here. As I mentioned in the
> call last week, it's great to see this coming together after years of
> banging our heads on the wall.
>
> Jeff
Glad to help,
Dusan
2026-07-14 Dusan Stojkovic <[email protected]>
PR target/123883
gcc/ChangeLog:
* match.pd: New pattern.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/pr123883.c: New test.
Co-authored-by: Jeff Law <[email protected]>
CONFIDENTIALITY: The contents of this e-mail are confidential and intended only
for the above addressee(s). If you are not the intended recipient, or the
person responsible for delivering it to the intended recipient, copying or
delivering it to anyone else or using it in any unauthorized manner is
prohibited and may be unlawful. If you receive this e-mail by mistake, please
notify the sender and the systems administrator at [email protected]
immediately.
---
gcc/match.pd | 21 ++++++++
gcc/testsuite/gcc.target/riscv/pr123883-a.c | 59 +++++++++++++++++++++
gcc/testsuite/gcc.target/riscv/pr123883-b.c | 58 ++++++++++++++++++++
3 files changed, 138 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/riscv/pr123883-a.c
create mode 100644 gcc/testsuite/gcc.target/riscv/pr123883-b.c
diff --git a/gcc/match.pd b/gcc/match.pd
index a7cec25dbad..521c62a3766 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -12335,6 +12335,27 @@ and,
&& @0 == @3)
(bit_xor (rrotate @0 @4) @2)))
+
+/* (T0)(1 << x) & (T1)(1 << x) -> (T)(1 << x),
+ * but keep the shift in the wider type to avoid introducing
+ * undefined behaviour. */
+(simplify
+ (bit_and
+ (convert? (lshift@2 integer_onep@1 @0))
+ (convert? (lshift@3 integer_onep@4 @0)))
+ (with
+ {
+ tree ltype0 = TREE_TYPE (@2);
+ tree ltype1 = TREE_TYPE (@3);
+ }
+ (if (INTEGRAL_TYPE_P (type)
+ && INTEGRAL_TYPE_P (ltype0)
+ && INTEGRAL_TYPE_P (ltype1)
+ && element_precision (type) <= element_precision (ltype0)
+ && element_precision (ltype0) <= element_precision (ltype1))
+ (convert:type
+ (lshift:ltype1 { @4; } @0)))))
+
/* Optimize extraction from a uniform vector to a representative element as
long as the requested element is within range. */
(simplify (IFN_VEC_EXTRACT (vec_duplicate @0) INTEGER_CST@1)
diff --git a/gcc/testsuite/gcc.target/riscv/pr123883-a.c
b/gcc/testsuite/gcc.target/riscv/pr123883-a.c
new file mode 100644
index 00000000000..39cc2f5ee09
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr123883-a.c
@@ -0,0 +1,59 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcb -mabi=lp64d" { target rv64 } } */
+/* { dg-options "-march=rv32gcb -mabi=ilp32" { target rv32 } } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-Og" "-Os" "-Oz" } } */
+
+#define TEST_STORE_NARROW_SHIFT(TYPE, WTYPE, NAME) \
+void \
+NAME (TYPE *data, unsigned int lo_bit) \
+{ \
+ WTYPE mask = (((WTYPE) 1UL << 1) - 1) << lo_bit; \
+ *data = (*data & ~mask) | ((1U << lo_bit) & mask); \
+}
+
+#define TEST_RET_NARROW_SHIFT(TYPE, WTYPE, NAME) \
+TYPE \
+NAME (TYPE data, unsigned int lo_bit) \
+{ \
+ WTYPE mask = (((WTYPE) 1 << 1) - 1) << lo_bit; \
+ return (data & ~mask) | ((1U << lo_bit) & mask); \
+}
+
+/* WTYPE = unsigned int */
+TEST_STORE_NARROW_SHIFT(unsigned char, unsigned int, store_uc_ui_narrow)
+TEST_STORE_NARROW_SHIFT(unsigned short, unsigned int, store_us_ui_narrow)
+
+TEST_RET_NARROW_SHIFT(unsigned char, unsigned int, ret_uc_ui_narrow)
+TEST_RET_NARROW_SHIFT(unsigned short, unsigned int, ret_us_ui_narrow)
+
+/* WTYPE = unsigned long */
+TEST_STORE_NARROW_SHIFT(unsigned char, unsigned long, store_uc_ul_narrow)
+TEST_STORE_NARROW_SHIFT(unsigned short, unsigned long, store_us_ul_narrow)
+TEST_STORE_NARROW_SHIFT(unsigned int, unsigned long, store_ui_ul_narrow)
+
+TEST_RET_NARROW_SHIFT(unsigned char, unsigned long, ret_uc_ul_narrow)
+TEST_RET_NARROW_SHIFT(unsigned short, unsigned long, ret_us_ul_narrow)
+TEST_RET_NARROW_SHIFT(unsigned int, unsigned long, ret_ui_ul_narrow)
+
+/* WTYPE = unsigned long long */
+TEST_STORE_NARROW_SHIFT(unsigned char, unsigned long long,
store_uc_ull_narrow)
+TEST_STORE_NARROW_SHIFT(unsigned short, unsigned long long,
store_us_ull_narrow)
+TEST_STORE_NARROW_SHIFT(unsigned int, unsigned long long,
store_ui_ull_narrow)
+
+TEST_RET_NARROW_SHIFT(unsigned char, unsigned long long, ret_uc_ull_narrow)
+TEST_RET_NARROW_SHIFT(unsigned short, unsigned long long, ret_us_ull_narrow)
+TEST_RET_NARROW_SHIFT(unsigned int, unsigned long long, ret_ui_ull_narrow)
+
+/* { dg-final { scan-assembler-times "\\tbset\\t" 14 { target rv64 } } } */
+/* Current remaining missed SI-mode cases. */
+/* { dg-final { scan-assembler-times "\\tsllw\\t" 4 { target rv64 } } } */
+/* { dg-final { scan-assembler-times "\\tbinv\\t" 2 { target rv64 } } } */
+/* { dg-final { scan-assembler-times "\\tsext.w\\t" 4 { target rv64 } } } */
+
+
+/* { dg-final { scan-assembler-times "\\tbset\\t" 16 { target rv32 } } } */
+/* Testcases with ULL. */
+/* { dg-final { scan-assembler-times "\\tbinv\\t" 2 { target rv32 } } } */
+/* { dg-final { scan-assembler-times "\\tand\\t" 8 { target rv32 } } } */
+/* { dg-final { scan-assembler-times "\\tsrai\\t" 6 { target rv32 } } } */
+
diff --git a/gcc/testsuite/gcc.target/riscv/pr123883-b.c
b/gcc/testsuite/gcc.target/riscv/pr123883-b.c
new file mode 100644
index 00000000000..4e623764228
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr123883-b.c
@@ -0,0 +1,58 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcb -mabi=lp64d" { target rv64 } } */
+/* { dg-options "-march=rv32gcb -mabi=ilp32" { target rv32 } } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-Og" "-Os" "-Oz" } } */
+
+#define TEST_STORE_NARROW_SHIFT(TYPE, WTYPE, NAME) \
+void \
+NAME (TYPE *data, unsigned int lo_bit) \
+{ \
+ WTYPE mask = ((1UL << 1) - 1) << lo_bit; \
+ *data = (*data & ~mask) | (((WTYPE) 1U << lo_bit) & mask); \
+}
+
+#define TEST_RET_NARROW_SHIFT(TYPE, WTYPE, NAME) \
+TYPE \
+NAME (TYPE data, unsigned int lo_bit) \
+{ \
+ WTYPE mask = ((1 << 1) - 1) << lo_bit; \
+ return (data & ~mask) | (((WTYPE)1U << lo_bit) & mask); \
+}
+
+/* WTYPE = unsigned int */
+TEST_STORE_NARROW_SHIFT(unsigned char, unsigned int, store_uc_ui_narrow)
+TEST_STORE_NARROW_SHIFT(unsigned short, unsigned int, store_us_ui_narrow)
+
+TEST_RET_NARROW_SHIFT(unsigned char, unsigned int, ret_uc_ui_narrow)
+TEST_RET_NARROW_SHIFT(unsigned short, unsigned int, ret_us_ui_narrow)
+
+/* WTYPE = unsigned long */
+TEST_STORE_NARROW_SHIFT(unsigned char, unsigned long, store_uc_ul_narrow)
+TEST_STORE_NARROW_SHIFT(unsigned short, unsigned long, store_us_ul_narrow)
+TEST_STORE_NARROW_SHIFT(unsigned int, unsigned long, store_ui_ul_narrow)
+
+TEST_RET_NARROW_SHIFT(unsigned char, unsigned long, ret_uc_ul_narrow)
+TEST_RET_NARROW_SHIFT(unsigned short, unsigned long, ret_us_ul_narrow)
+TEST_RET_NARROW_SHIFT(unsigned int, unsigned long, ret_ui_ul_narrow)
+
+/* WTYPE = unsigned long long */
+TEST_STORE_NARROW_SHIFT(unsigned char, unsigned long long,
store_uc_ull_narrow)
+TEST_STORE_NARROW_SHIFT(unsigned short, unsigned long long,
store_us_ull_narrow)
+TEST_STORE_NARROW_SHIFT(unsigned int, unsigned long long,
store_ui_ull_narrow)
+
+TEST_RET_NARROW_SHIFT(unsigned char, unsigned long long, ret_uc_ull_narrow)
+TEST_RET_NARROW_SHIFT(unsigned short, unsigned long long, ret_us_ull_narrow)
+TEST_RET_NARROW_SHIFT(unsigned int, unsigned long long, ret_ui_ull_narrow)
+
+/* { dg-final { scan-assembler-times "\\tbset\\t" 16 { target rv64 } } } */
+/* { dg-final { scan-assembler-not "\\tsllw\\t" { target rv64 } } } */
+/* { dg-final { scan-assembler-not "\\tbinv\\t" { target rv64 } } } */
+/* { dg-final { scan-assembler-not "\\txor\\t" { target rv64 } } } */
+/* { dg-final { scan-assembler-not "\\tand\\t" { target rv64 } } } */
+
+/* { dg-final { scan-assembler-times "\\tbset\\t" 16 { target rv32 } } } */
+/* Testcases with ULL. */
+/* { dg-final { scan-assembler-times "\\tand\\t" 7 { target rv32 } } } */
+/* { dg-final { scan-assembler-times "\\tbclr\\t" 5 { target rv32 } } } */
+/* { dg-final { scan-assembler-times "\\tsrai\\t" 6 { target rv32 } } } */
+
--
2.43.0
