On Thu, Aug 14, 2025 at 3:43 PM Jeff Law <jeffreya...@gmail.com> wrote: > > While working to remove mvconst_internal I stumbled over a regression in > the code to handle signed division by a power of two. > > In that sequence we want to select between 0, 2^n-1 by pairing a sign > bit splat with a subsequent logical right shift. This can be done > without branches or conditional moves. > > Playing with it a bit made me realize there's a handful of selections we > can do based on a sign bit test. Essentially there's two broad cases. > > Clearing bits after the sign bit splat. So we have 0, -1, if we clear > bits the 0 stays as-is, but the -1 could easily turn into 2^n-1, ~2^n-1, > or some small constants. > > Setting bits after the sign bit splat. If we have 0, -1, setting bits > the -1 stays as-is, but the 0 can turn into 2^n, a small constant, etc. > > Shreya and I originally started looking at target patterns to do this, > essentially discovering conditional move forms of the selects and > rewriting them into something more efficient. That got out of control > pretty quickly and it relied on if-conversion to initially create the > conditional move. > > The better solution is to actually discover the cases during > if-conversion itself. That catches cases that were previously being > missed, checks cost models, and is actually simpler since we don't have > to distinguish between things like ori and bseti, instead we just emit > the natural RTL and let the target figure it out. > > In the ifcvt implementation we put these cases just before trying the > traditional conditional move sequences. Essentially these are a last > attempt before trying the generalized conditional move sequence. > > This as been bootstrapped and regression tested on aarch64, riscv, > ppc64le, s390x, alpha, m68k, sh4eb, x86_64 and probably a couple others > I've forgotten. It's also been tested on the other embedded targets. > Obviously the new tests are risc-v specific, so that testing was > primarily to make sure we didn't ICE, generate incorrect code or regress > target existing specific tests. > > Raphael has some changes to attack this from the gimple direction as > well. I think the latest version of those is on me to push through > internal review. > > OK for the trunk?
Apart from a missing ChangeLog this LGTM. I would expect this to also trigger on x86, so I wonder what effect the patch has there on the testcases you add for riscv? Thanks, Richard. > > Jeff
