On Sun, Sep 13, 2026 at 1:58 PM Andrea Pinski <[email protected]> wrote: > > On Sun, Sep 13, 2026 at 12:58 PM Jeff Law <[email protected]> > wrote: > > > > > > > > On 9/12/26 1:24 AM, Andrea Pinski wrote: > > > This adds a simplification for `x+umax(x,1)` to `umax(x*2, 1)` when `x*2` > > > is known not to overflow. This shows up with std::vector::push_back > > > after the patch which adds `x+minmax(a,b)` simplification; it is also > > > a special case of that patch for `umax and 1`. This gives a small > > > code generation improvement with std::vector::push_back because we > > > trade an add after the umax with a shift beforehand. > > > > > > Bootstrapped and tested on x86_64-linux-gnu. > > > > > > PR tree-optimization/127103 > > > > > > gcc/ChangeLog: > > > > > > * match.pd (`x+umax(x,1)`): New pattern. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gcc.dg/tree-ssa/max1-plus-1.c: New test. > > > > > > Signed-off-by: Andrea Pinski <[email protected]> > > Not objecting, more a general question. It seems like this is just > > trading one form for another. We haven't dropped the number of > > expression evaluations nor have we reduced the height of the dependency > > tree. So this feels more likely a canonicalizatino pattern or a pattern > > designed to change forms to make other optimizations more effective. Is > > that a fair characterization? > > It is canonicalization to some extent. It is more of a reassociation > really. In a similar way `a+(a+b)` and `a+(a*b)` are handled. > Note The general form of this is `x+max<a, b>` can be transformed into > `max<a+x, b+x>` (assuming both `a+x` and `b+x` have no overflow). In > the case of b being 1, the `x+1` can become 1 because we know that if > x*2 is 0, x+1 will be 1, otherwise `x+1 >= x*2` (with no overflow > which is checked upon) so the only value that matters is 1. > > Also even though we don't reduce the height of dependencies, the add > (shift) is simpler than a max expression so having it first will allow > for more issuing first. > > Note the second patch is enough to remove the branch so this one is > less of an important to me.
I just figured out another reason why we should do this. With the two patches I posted (will be fixing up the second one in a little bit), we get: ``` _83 = _36 * 2; _66 = MAX_EXPR <_83, 1>; _40 = MIN_EXPR <_66, 2305843009213693951>; // 2305843009213693951 = INT64_MAX/4 _50 = _40 * 4; ``` with std::vector<int>::push_back: Here if we swap MAX and MIN, we can form a SAT_MULT (signed) by 8 and then just a MAX_EXPR. For std::vector<char>::push_back we get: ``` _30 = _34 * 2; _14 = MAX_EXPR <_30, 1>; _15 = MIN_EXPR <_14, 9223372036854775807>; ``` Which is really: ``` ssize_t _t = (ssize_t) _34; _t1 = SAT_ADD (_t, _t); size_t _t2 = (size_t)_t1; _15 = MAX_EXPR<_t2, 1>; ``` Thanks, Andrea Pinski > > > > > Jeff > > > >
