On Tue, 8 Sep 2026, Kyrylo Tkachov wrote:
>
>
> > On 8 Sep 2026, at 15:21, Richard Biener <[email protected]> wrote:
> >
> > On Mon, 7 Sep 2026, [email protected] wrote:
> >
> >> From: Kyrylo Tkachov <[email protected]>
> >>
> >> genmatch can leave the iterators of a named match unexpanded. Code
> >> generation
> >> emits COND_EXPR conditions in opcode switches. Overlapping delayed
> >> iterator
> >> sets can then emit duplicate case labels.
> >
> > Can you show an example pattern?
>
> Patch 3/3 is enabled by this. I want to write:
> (for cmp (ge gt)
> (match (unsigned_integer_sat_add @0 @1)
> (cond^ (cmp:c (bit_not @1) @0)
> (plus:c @0 @1) integer_minus_onep)))
>
> (for cmp (lt le)
> (match (unsigned_integer_sat_add @0 @1)
> (cond^ (cmp:c (bit_not @1) @0)
> integer_minus_onep (plus:c @0 @1))))
>
> The :c modifier also generates the comparison with swapped operands:
> • (ge gt) produces direct GE/GT and commuted LE/LT.
> • (lt le) produces direct LT/LE and commuted GT/GE.
>
> Without this patch genmatch emits this:
Hmm. But it seems this is maybe an unfortunate interaction
between for substitution delaying and :c lowering? That is,
how is this special to (cond ...)? It seems to me the
real issue is elsewhere (some sorting issue?)
On a related note it seems that lower_cond should no longer
be necessary -- there is no GENERIC comparison operator
in [VEC_]COND_EXPR anymore.
Richard.
> switch (gimple_assign_rhs_code (_a2))
> {
> case GE_EXPR:
> case GT_EXPR:
> { ... }
>
> case LE_EXPR:
> case LT_EXPR:
> { ... }
>
> case LT_EXPR: /* Duplicate. */
> case LE_EXPR: /* Duplicate. */
> { ... }
>
> case GT_EXPR: /* Duplicate. */
> case GE_EXPR: /* Duplicate. */
> { ... }
> }
>
> Thanks,
> Kyrill
>
> >
> >> Substitute a group before decision-tree insertion when one of its iterators
> >> occurs in a COND_EXPR condition. The existing eager path keeps the
> >> substitutions aligned and lets the decision tree merge common concrete
> >> operations. Other named matches keep delayed substitution.
> >>
> >> A reduced GIMPLE and GENERIC matcher with overlapping iterator sets fails
> >> to
> >> compile before this change. Both forms compile after the change.
> >> This is needed to accept the patterns in patch 3/3.
> >>
> >> Bootstrapped and tested on aarch64-none-linux-gnu.
> >>
> >> Ok for trunk?
> >> Thanks,
> >> Kyrill
> >>
> >> gcc/ChangeLog:
> >>
> >> * genmatch.cc (cond_expr_condition_contains_id): New function.
> >> (lower_for): Expand iterators used in COND_EXPR conditions.
> >> (lower): Update comment.
> >>
> >> Signed-off-by: Kyrylo Tkachov <[email protected]>
> >> ---
> >> gcc/genmatch.cc | 44 ++++++++++++++++++++++++++++++++++++++++----
> >> 1 file changed, 40 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> >> index 1bc5875d7dd..c4bb1c12400 100644
> >> --- a/gcc/genmatch.cc
> >> +++ b/gcc/genmatch.cc
> >> @@ -2159,6 +2159,26 @@ contains_id (operand *o, user_id *id)
> >> return false;
> >> }
> >>
> >> +/* Return true if O contains a COND_EXPR condition that refers to ID. */
> >> +
> >> +static bool
> >> +cond_expr_condition_contains_id (operand *o, user_id *id)
> >> +{
> >> + if (capture *c = dyn_cast <capture *> (o))
> >> + return c->what && cond_expr_condition_contains_id (c->what, id);
> >> +
> >> + if (expr *e = dyn_cast <expr *> (o))
> >> + {
> >> + if (*e->operation == COND_EXPR && contains_id (e->ops[0], id))
> >> + return true;
> >> + for (unsigned i = 0; i < e->ops.length (); ++i)
> >> + if (cond_expr_condition_contains_id (e->ops[i], id))
> >> + return true;
> >> + }
> >> +
> >> + return false;
> >> +}
> >> +
> >>
> >> /* In AST operand O replace operator ID with operator WITH. */
> >>
> >> @@ -2304,9 +2324,24 @@ lower_for (simplify *sin, vec<simplify *>&
> >> simplifiers)
> >> else
> >> can_delay_subst = false;
> >> }
> >> - if (sin->kind == simplify::MATCH
> >> - && can_delay_subst)
> >> - continue;
> >> + /* Code generation emits COND_EXPR conditions in opcode switches.
> >> + Substitute iterators in these conditions before inserting the patterns
> >> + into the decision tree. Overlapping iterator sets could otherwise
> >> + emit duplicate case labels. */
> >> + if (sin->kind == simplify::MATCH && can_delay_subst)
> >> + {
> >> + for (unsigned si = worklist_start;
> >> + si < worklist.length () && can_delay_subst; ++si)
> >> + for (unsigned i = 0; i < n_ids; ++i)
> >> + if (cond_expr_condition_contains_id (worklist[si]->match,
> >> + ids[i]))
> >> + {
> >> + can_delay_subst = false;
> >> + break;
> >> + }
> >> + if (can_delay_subst)
> >> + continue;
> >> + }
> >>
> >> unsigned worklist_end = worklist.length ();
> >> for (unsigned si = worklist_start; si < worklist_end; ++si)
> >> @@ -2374,7 +2409,8 @@ lower (vec<simplify *>& simplifiers, bool gimple)
> >> /* Lower for needs to happen before lowering cond
> >> to support (for cnd (cond vec_cond)). This is
> >> safe as substitution delay does not happen for
> >> - cond or vec_cond. */
> >> + cond or vec_cond, or for iterators used in a
> >> + COND_EXPR condition. */
> >> out_simplifiers.truncate (0);
> >> for (auto s: simplifiers)
> >> lower_for (s, out_simplifiers);
> >>
> >
> > --
> > Richard Biener <[email protected]>
> > SUSE Software Solutions Germany GmbH,
> > Frankenstrasse 146, 90461 Nuernberg, Germany;
> > GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)
>
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)
