On 7/10/2026 9:18 AM, Dusan Stojkovic wrote: > > Thanks, the updated pattern includes all of this. > > > if you want a future expansion on this, a nice change would remove the > > `:c` and add a check for the other way around; that is `type <= > > itype0 <= itype1` and using itype1 for the inner type. > > I removed the `:c` and arrived at the pattern in the updated patch. > First I added a new test which swaps the placement of the WTYPE > cast. The pattern catches these cases as well somewhat. > Additionally, the tests try to cast to int are added now as well. > I also tried combinations in which I swap the shifts, ands and ors around > in the test case, but this produced the same GIMPLE in the optimized pass. > That is why `:c` is dropped entirely. 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. > > 2026-10-07 Dusan Stojkovic <[email protected]> > > PR target/123883 > > gcc/ChangeLog: > > * match.pd: New pattern. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/pr123883-a.c: New test. > * gcc.target/riscv/pr123883-b.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 | 22 ++++++++ > gcc/testsuite/gcc.target/riscv/pr123883-a.c | 59 +++++++++++++++++++++ > gcc/testsuite/gcc.target/riscv/pr123883-b.c | 58 ++++++++++++++++++++ > 3 files changed, 139 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..0de4addf44a 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -12335,6 +12335,28 @@ 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); > + tree largertype = ltype0; > + } > + (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:largertype { @1; } @0))))) 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. > > + > +/* { dg-final { scan-assembler-times "bset" 14 { target rv64 } } } */ > +/* Current remaining missed SI-mode cases. */ > +/* { dg-final { scan-assembler-times "sllw" 4 { target rv64 } } } */ > +/* { dg-final { scan-assembler-times "binv" 2 { target rv64 } } } */ > +/* { dg-final { scan-assembler-times "sext.w" 4 { target rv64 } } } */ > + > + > +/* { dg-final { scan-assembler-times "bset" 16 { target rv32 } } } */ > +/* Testcases with ULL. */ > +/* { dg-final { scan-assembler-times "xor" 2 { target rv32 } } } */ > +/* { dg-final { scan-assembler-times "and\\t" 2 { target rv32 } } } */ > +/* { dg-final { scan-assembler-times "srai" 1 { target rv32 } } } */ 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. 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
