On Wed, 14 Oct 2026, Jeff Law wrote: > > > On 9/12/26 7:14 AM, Dominic P wrote: > > Video codecs spell a clamp of a signed value to [0, M] as > > (x & ~M) ? ((-x) >> (prec - 1)) & M : x, which the front end folds to > > (unsigned) x > M ? ((-x) >> (prec - 1)) & M : x. That is never turned > > into MIN/MAX, so it if-converts into four vector operations where the > > plain x < 0 ? 0 : x > M ? M : x spelling gives smax/smin. > > > > The two forms agree except at x == INT_MIN, where -x is undefined, so > > that arm is restricted to types with undefined signed overflow. > > > > FFmpeg spells the same clamp with the complement instead of the negation, > > (x & ~M) ? ((~x) >> (prec - 1)) & M : x (av_clip_uintp2, > > libavutil/common.h), > > while x264 uses the negation (x264_clip_pixel, common/common.h). ~x is > > negative exactly when x is not, so it splats the same sign; it is total, > > so that arm carries no overflow guard at all and stays correct under > > -fwrapv and -ftrapv. Both are now one pattern. > > > > av_clip_uintp2 also returns unsigned rather than the type of x, so the > > result and the unchanged arm can carry a conversion to the type the range > > test already uses; the pattern accepts it and the guards read the type of x. > > Without that the rule cannot fire on any of FFmpeg's nine, ten, twelve and > > fourteen bit pixel writes, which is where the whole win is: aarch64 has no > > assembly for those depths. Recompiling FFmpeg's h264 and hevc DSP with and > > without this patch, nothing else changed, .text falls by 6,892 bytes at -O2 > > and by 16,416 bytes at -O3, with sao_edge_restore_0_10 going from 3,252 to > > 1,588 bytes and pred8x8_plane_10_c from 1,308 to 512; h264qpel.c grows by > > 4,480 bytes at -O3, because a cheaper clamp changes what the vectorizer > > decides to do around it. Executed instructions for the ten-bit weighted > > prediction and residual add, counted under qemu with one instruction per > > translation block, fall from 10,464,997 to 6,444,997. > > > > An exhaustive model of the guard at eight, ten and twelve bits, over every > > threshold, every mask and every x, reports no unsound acceptance for either > > spelling; the complement arm also has no sound pair that the guard rejects, > > where the negation arm has one per width. > > > > The threshold may be M or M - 1. The masked arm is M for every x > 0, > > so at x == M it is M, as the clamp is; (unsigned) x > M - 1 therefore > > selects the same values as (unsigned) x > M. M - 1 is how the > > (unsigned) x >= M spelling arrives, since >= and < against a positive > > constant are canonicalized to > and <= against the constant less one. > > The <= spelling with the arms exchanged reaches this pattern as the > > > form: fold_ternary_loc swaps the arms of a COND_EXPR on GENERIC and > > phiopt retries every match with the inverted comparison on GIMPLE. > > > > This is the neighbour of the conditional narrow clip added for PR127166: > > that one narrows to a smaller type, this one clips at the width of x and > > takes the mask the idiom applies to the shift. Spelled on a sub-int > > type the idiom does not match, as the negation is performed on the > > promoted int, and a mask of 1 does not reach it either, since > > ((-x) >> (prec - 1)) & 1 is rewritten to (unsigned) x >> (prec - 1) > > first. Vectors do not reach the pattern: it matches a COND_EXPR > > against INTEGER_CST thresholds with an integral result type, which a > > vector spelling never is; for the negation the masked arm is in any > > case rewritten to a compare mask by the (-x >> C) fold first. The > > pending "Saturating arithmetic matching improvements" series (Kyrylo > > Tkachov, 2026-09-02) is disjoint: its clamp forms all narrow and end in > > a .SAT_* call; this rule works at the width of x. > > > > Bootstrapped and regression tested on x86_64-pc-linux-gnu. > > > > Assisted-by: Claude Opus 5 (Anthropic) > > > > gcc/ChangeLog: > > > > * match.pd ((unsigned) x > M ? ((-x) >> (prec - 1)) & M : x) > > ((unsigned) x > M ? ((~x) >> (prec - 1)) & M : x): New > > simplification for both spellings of the sign splat. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.dg/tree-ssa/clip-canon-1.c: New test. > > * gcc.dg/tree-ssa/clip-canon-2.c: New test. > > * gcc.dg/tree-ssa/clip-canon-3.c: New test. > > * gcc.dg/tree-ssa/clip-canon-4.c: New test. > > * gcc.dg/tree-ssa/clip-canon-5.c: New test. > > * gcc.dg/tree-ssa/clip-canon-6.c: New test. > > * gcc.dg/tree-ssa/clip-canon-7.c: New test. > > * gcc.dg/tree-ssa/clip-canon-8.c: New test. > > * gcc.dg/tree-ssa/clip-canon-9.c: New test. > > > > Signed-off-by: Dominic P <[email protected]> > > --- > > Andrea Pinski -- v6 extends the rule rather than changing what v5 did. > > > > - Second spelling of the sign splat. x264 writes the clamp with a > > negation, FFmpeg's av_clip_uintp2 with ~x. ~x is negative exactly when > > x is not, and it is total, so that arm needs neither the > > undefined-overflow nor the no-sanitize guard and stays correct under > > -fwrapv and -ftrapv. One (for splat (negate bit_not)) covers both. > > > > - av_clip_uintp2 returns unsigned rather than the type of x, so the arms > > may carry a conversion to the type the range test already uses; v5 > > could not fire there at all. The guards now read the type of x, and a > > new !TYPE_UNSIGNED on it is what stops the complement arm matching an > > unsigned x, where the shift is logical (clip-canon-9.c). > > > > - That is where the numbers are: those are the 9, 10, 12 and 14 bit pixel > > writes, and aarch64 has no assembly for them. FFmpeg's h264 and hevc > > DSP loses 6,892 bytes of .text at -O2 and 16,416 at -O3 (h264qpel.c > > gains 4,480 at -O3, disclosed in the message); executed instructions > > for the ten-bit weighted prediction and residual add fall 38 %. > > > > - I took your three functions as the gt, ge/lt and (x & ~255) != 0 > > spellings, written out in clip-canon-5.c. The two literal spellings > > stay in clip-canon-6.c as must-not-fold cases, since x & 255 and the > > reversed arms each compute a different function (x & 255 returns 256 at > > x == 256). If you meant either literally, say so and I will follow. > > > > - Still no separate le pattern: I could not make one fire, and a cc1 with > > one gives dumps identical to one without; say the word and I will write > > it out anyway. The exhaustive model of the guard was re-run over both > > spellings: no unsound acceptance either way. > > > > > > > > diff --git a/gcc/match.pd b/gcc/match.pd > > index 032f29dcbc2..d6cde161089 100644 > > --- a/gcc/match.pd > > +++ b/gcc/match.pd > > @@ -13390,6 +13390,36 @@ and, > > (max @0 { build_zero_cst (TREE_TYPE (@0)); })) > > @1))))))) > > +/* The clip of PR127166 at the width of X, with the mask spelled out and > > the > > + sign of X splat either way: > > + > > + (UT) X > M ? ((-X) >> (PREC - 1)) & M : X -> MAX (MIN (X, M), 0) > > + (UT) X > M ? ((~X) >> (PREC - 1)) & M : X -> MAX (MIN (X, M), 0). > > + > > + The mask is M at X == M, so a threshold of M - 1, which is how > > + (UT) X >= M is canonicalized, selects the same values. ~X is negative > > + exactly when X is not, so it splats the same sign, and unlike -X it is > > + defined at the minimum of the type, so only the negation needs the > > + overflow guards. The result may be converted to the unsigned type the > > + range test uses instead of keeping the type of X. */ > > +(for splat (negate bit_not) > > + (simplify > > + (cond (gt (nop_convert@2 @0) INTEGER_CST@1) > > + (nop_convert? (bit_and:s (rshift (splat @0) INTEGER_CST@4) > > + INTEGER_CST@5)) > > + (nop_convert? @0)) > > + (if (INTEGRAL_TYPE_P (type) > > + && !TYPE_UNSIGNED (TREE_TYPE (@0)) > > + && TYPE_UNSIGNED (TREE_TYPE (@2)) > > + && (splat == BIT_NOT_EXPR > > + || (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)) > > + && !TYPE_OVERFLOW_SANITIZED (TREE_TYPE (@0)))) > > + && wi::to_wide (@4) == TYPE_PRECISION (TREE_TYPE (@0)) - 1 > > + && tree_int_cst_sgn (@5) > 0 > > + && (wi::to_wide (@1) == wi::to_wide (@5) > > + || wi::to_wide (@1) + 1 == wi::to_wide (@5))) > > + (convert (max (min @0 @5) { build_zero_cst (TREE_TYPE (@0)); }))))) > So don't you have to worry that @5 is a different type than @0 since this > works on generic as well as gimple?
They should be the "same" (TYPE_MAIN_VARIANT) in GENERIC and compatible in GIMPLE, so no need to worry. Unless 'splat' folds in conversions? > And a comment nit. You indicate that ~X is negative when X is not. But > consider if X is zero ;-) I think the pattern still works, so it's just a > comment nit. > > LLM review suggests more run tests to confirm proper behavior, particularly > the ~x forms. But I'd consider that nice to have, not a hard requirement. > > LLM review pointed out that clip-canon-8 has a positive and a negative test > and that it's impossible to tell in the scan step if it was the correct or > incorrect transformation that triggered. Similarly for clip-canon-9. Consider > breaking the positive and negative tests out distinctly. Not required, but > again, nice to have. > > LLM also suggests a -ftrapv test for the ~x tests. Nice to have, but not > required and it suggested testing at INT_MIN. > > So OK for the trunk after resolving the type of @5 question in one way or > another. Your call on whether or not to improve the tests and how to do so if > you choose to in this patch or a follow-up. > > Jeff > > -- Richard Biener <[email protected]> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri; (HRB 36809, AG Nuernberg)
