On Tue, Sep 8, 2026 at 7:07 AM Richard Biener <[email protected]> wrote: > > 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.
For most of the time that is true BUT phiopt still produces COND_EXPR with a generic comparison operator inside a cond_expr. I have a start of the cleanup there but didn't finish it yet. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117385 . Now I am getting closer on other stuff I might go and try to finish that up. Note fold_build_cond_expr in tree-ifconv.cc needs the same treatment as phiopt and I have not looked into that yet. Thanks, Andrea > > 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)
