Mostly looks reasonable, but I see a couple things worth calling out. ### Main correctness concern The `IFN_CLZ/IFN_CTZ` pattern seems too permissive:
```c (if (types_match (@0, @1) && wi::geu_p (wi::to_wide (@2), TYPE_PRECISION (TREE_TYPE (@0)) - 1)) (func (bit_ior @0 @1) @2)) ``` For the 2-arg generic forms, the second argument is the value returned for zero. The identity - `min(ctzg(x, z), ctzg(y, z)) == ctzg(x|y, z)` - `min(clzg(x, z), clzg(y, z)) == clzg(x|y, z)` only holds when `z` is at least the **full precision**, not `precision - 1`. Counterexample for `ctzg` with 32-bit type and `z = 31`: - `x = 0`, `y = 1` - `ctzg(0,31) = 31` - `ctzg(1,31) = 0` - `min(...) = 0` - `x|y = 1`, so `ctzg(1,31) = 0` — fine here But for `clzg`: - `x = 0`, `y = 0x80000000` - `clzg(0,31) = 31` - `clzg(0x80000000,31) = 0` - `min(...) = 0` - `x|y = 0x80000000`, result `0` — also fine The real problematic case is both zero: - `min(func(0,z), func(0,z)) = z` - `func(0|0,z) = z` So the above examples don't fail, but the guard still looks suspicious. The mathematically natural bound is `z >= precision`, because ordinary `clz/ctz` on nonzero values range from `0..precision-1`, and you want zero's fallback not to become smaller than any nonzero result. With `z = precision-1`, zero becomes indistinguishable from the maximal nonzero result, which may still be OK for `min`, but I'd want this justified explicitly. As written, this is subtle enough that it deserves either a proof in the comment or tightening to `>= precision` if that was the actual intent. On 6/28/2026 11:26 PM, Eikansh Gupta wrote: > This adds match.pd simplifications for min(clz(x), clz(y)) and > min(ctz(x), ctz(y)) to clz(x | y) and ctz(x | y). > > PR tree-optimization/123311 > > gcc/ChangeLog: > > * match.pd (min(clz(x), clz(y)) -> clz(x | y)): New pattern. > (min(ctz(x), ctz(y)) -> ctz(x | y)): Likewise. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/pr123311.c: New test. Don't we have to test the CLZ_DEFINED_VALUE_AT_ZERO and CTZ_DEFINED_VALUE_AT_ZERO? > > +/* min (clz (x), clz (y)) -> clz (x | y) and > + min (ctz (x), ctz (y)) -> ctz (x | y). */ > +(for func (CLZ CTZ) > + (simplify > + (min (func:s @0) (func:s @1)) > + (if (types_match (@0, @1) > + && (!sanitize_flags_p (SANITIZE_BUILTIN) > + || (cfun && (cfun->curr_properties & PROP_ssa) != 0))) Hmm. Can you explain what these two trailing conditions are doing as a comment in this patch? I'm guessing you want to give the sanitizers a chance to instrument the builtin? But better to be sure. > + (func (bit_ior @0 @1))))) > +(for func (IFN_CLZ IFN_CTZ) > + (simplify > + (min (func:s @0 INTEGER_CST@2) (func:s @1 @2)) > + (if (types_match (@0, @1) > + && wi::geu_p (wi::to_wide (@2), > + TYPE_PRECISION (TREE_TYPE (@0)) - 1)) > + (func (bit_ior @0 @1) @2)))) And why don't we need to do the same or this pattern? Mostly looks good, just those few nits to chase down. Jeff
