On Tue, 6 Oct 2026, Kyrylo Tkachov wrote: > > > > On 6 Oct 2026, at 15:34, Richard Biener <[email protected]> wrote: > > > > On Tue, 6 Oct 2026, Kyrylo Tkachov wrote: > > > >> > >> > >>> On 6 Oct 2026, at 13:59, Richard Biener <[email protected]> wrote: > >>> > >>> On Tue, 6 Oct 2026, [email protected] wrote: > >>> > >>>> From: Kyrylo Tkachov <[email protected]> > >>>> > >>>> lower_for replicates each pattern once per operator of each for-level, > >>>> substituting the operator into the match and result. When a pattern > >>>> does not reference any of the for-level iterators in either its match > >>>> or result, every substituted copy is structurally identical, producing > >>>> dead leaves in the decision tree. > >>>> > >>>> Since f6eb8291e1c (PR119196, 2025-07-09), match.pd:6638 has a > >>>> (for lop ...) pattern nested inside a (for op ...) whose 34-operator > >>>> list it does not use. lower_for therefore emits 34 identical copies of > >>>> each of the three lop alternatives per GIMPLE and GENERIC IL. > >>>> > >>>> Before proceeding through the operator substitution loop, check whether > >>>> any id in the current for-level appears in the pattern's match or > >>>> result. If none do, a single copy with j=0 operator values suffices > >>>> and the remaining max_n_opers-1 iterations are skipped. > >>>> > >>>> Bootstrapped and tested on aarch64-none-linux-gnu. > >>>> Ok for trunk? > >>> > >>> Might there be any valid uses of such case? It looks like the pattern > >>> in question should be simply moved out of the enclosing (for ...). > >>> > >>> That said, this should emit an unconditional diagnostic from genmatch. > >>> > >>> Can you re-post with such a change (and move the pattern)? > >>> > >> > >> Yeah, that works. > >> Does this version look ok? > > > > Looking I wonder what happens to a non-sensical case like > > > > (for opA (plus minus) > > (for opB (minus minus) > > (simplify (opB @0 @1) > > (if (opA == minus) > > (opA @0 @1))))) > > > > so whether this should be an error instead? ISTR we did have > > sth like > > > > (for icmp (tcc_comparison) > > (for cmp (tcc_comparison) > > (simplify (bit_not (cmp @0 @1)) > > (with > > { > > enum tree_code icmpc = invert_tree_comparison (...); > > } > > (if (icmpc == icmp) > > (icmp @0 @1)))) > > > > or so, but we can now handle this better thanks to Andrew. But in > > general not all such cases might be fixed, so do we need to verify > > the iterator isn't used in the replacement as well? For the above > > case a diagnostic would be still OK since we now have the extended > > (with ...) syntax that can avoid the outer iteration. > > > > Yes, the check covers the replacement as well as the match: > > any_id_used = (contains_id (s->match, ids[i]) > || (s->result && contains_id (s->result, ids[i]))); > > The diagnostic only fires when the iterator appears in neither. If the > iterator is used only in an inline C predicate or guard within the > replacement, patch 4/6 (which I’ve committed) ensures contains_id detects > that via a CPP_NAME token scan, so the full expansion still proceeds > correctly.
The patch is OK then; we might want to issue a separate diagnostic when the iterator is only used in s->result (a more efficient way to write the pattern should be available). Thanks, Richard. > Thanks, > Kyrill > > > Richard. > > > >> Thanks, > >> Kyrill > >> > >> > >>> Thanks, > >>> Richard. > >>> > >>>> Thanks, > >>>> Kyrill > >>>> > >>>> gcc/ChangeLog: > >>>> > >>>> * genmatch.cc (lower_for): Skip replicated substitutions when no > >>>> for-level iterator appears in the pattern. > >>>> > >>>> Signed-off-by: Kyrylo Tkachov <[email protected]> > >>>> --- > >>>> gcc/genmatch.cc | 8 +++++++- > >>>> 1 file changed, 7 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc > >>>> index 9bb7cbbb427..1d53283e094 100644 > >>>> --- a/gcc/genmatch.cc > >>>> +++ b/gcc/genmatch.cc > >>>> @@ -2433,7 +2433,13 @@ lower_for (simplify *sin, vec<simplify *>& > >>>> simplifiers) > >>>> for (unsigned si = worklist_start; si < worklist_end; ++si) > >>>> { > >>>> simplify *s = worklist[si]; > >>>> - for (unsigned j = 0; j < max_n_opers; ++j) > >>>> + /* If none of the for-level ids appear in this pattern's match or > >>>> + result, all operator substitutions are no-ops. Emit one copy. */ > >>>> + bool any_id_used = false; > >>>> + for (unsigned i = 0; i < n_ids && !any_id_used; ++i) > >>>> + any_id_used = (contains_id (s->match, ids[i]) > >>>> + || (s->result && contains_id (s->result, ids[i]))); > >>>> + for (unsigned j = 0; j < (any_id_used ? max_n_opers : 1u); ++j) > >>>> { > >>>> operand *match_op = s->match; > >>>> operand *result_op = s->result; > >>>> > >>> > >>> -- > >>> 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) > >> > >> > >> > > > > -- > > 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) > > > -- 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)
