On Thu, May 22, 2025 at 3:11 AM Jeff Law <jeffreya...@gmail.com> wrote: > > > > On 5/18/25 10:38 AM, Andrew Pinski wrote: > > While working on improving forwprop and removal of > > forward_propagate_into_gimple_cond/forward_propagate_into_comparison, > > I came cross a case where we end up with SSA_NAME in the resulting > > gimple_match_op and one statement in the sequence. This was the result > > of simplification of: > > ``` > > _3 = MIN_EXPR <maxlen_2(D), 264> > 16 > > if (_3 > 16) ... > > ``` > > > > Which simplifies down to: > > (maxlen_2(D) > 16) & (264 > 16) > > into > > (maxlen_2(D) > 16) & 1 > > > > Which `maxlen_2(D) > 16` gets pushed onto the sequence > > and then the & 1 is removed via the match pattern: > > ``` > > /* x & ~0 -> x */ > > (simplify > > (bit_and @0 integer_all_onesp) > > (non_lvalue @0)) > > ``` > > > > So what this patch does is to undo the push extracting the new op > > from the pushed statement and remove the sequence as it is not used > > any more. > > > > Bootstrapped and tested on x86_64-linux-gnu. > > > > gcc/ChangeLog: > > > > PR tree-optimization/120331 > > * gimple-match-exports.cc (maybe_undo_push): New function. > > (gimple_simplify): Call maybe_undo_push if resimplify was successfull. > OK, but this isn't my strongest area. So give Richi until Monday AM to > chime in if he's got concerns.
It's a bit ugly and IMO it should be in-line at the single user and have an explaining comment why we still might see such cases. I'll note that this does not help when match recursively applies simplifications or when 'seq' is passed as NULL. So addressing this in the genmatch generated code directly would be superior (but IIRC what the bug said this isn't possible?) Richard. > Thanks, > jeff >
