On 7/14/2026 8:19 AM, Dusan Stojkovic wrote:
> 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]>
This looks good. I guess the only question is whether or not to add the
:c to the bit_and to allow it to match both forms. I realize that
without a testcase we're not going to test the alternate form, but
looking at the pattern I don't see any real reason for the other form to
show up. So I'm going to include the :c.
I'll do another bootstrap & regression test on x86-64 since mine ran
without the :c. I'll also do spot testing of riscv32-elf and
riscv64-elf. Assuming that's clean, I'll push it later today.
jeff
