> -----Original Message-----
> From: Tamar Christina <[email protected]>
> Sent: 13 November 2025 13:37
> To: Richard Biener <[email protected]>
> Cc: [email protected]; nd <[email protected]>
> Subject: RE: [PATCH 1/2]middle-end: add target hook for isel
>
> > -----Original Message-----
> > From: Richard Biener <[email protected]>
> > Sent: 13 November 2025 13:16
> > To: Tamar Christina <[email protected]>
> > Cc: [email protected]; nd <[email protected]>
> > Subject: Re: [PATCH 1/2]middle-end: add target hook for isel
> >
> > On Thu, 13 Nov 2025, Tamar Christina wrote:
> >
> > > This adds a new target hook to gimple-sel to allow targets to
> > > do target specific massaging of the gimple IL to prepare for
> > > expand.
> > >
> > > Tejas will be sending up part 2 of this soon to help convert
> > > SVE packed boolean VEC_COND_EXPR into something that we can
> > > handle more efficiently if expanded in a different order.
> > >
> > > We also want to use this to e.g. for Adv. SIMD prefer avoiding
> > > != vector compare expressions because the ISA doesn't have
> > > this instruction and so we expand to == + ~ but changing the
> > > expression from a MIN to MAX only for VECTOR_BOOLEAN_TYPE_P
> > > and flipping the operands we can expand more efficient.
> > >
> > > These are the kind of things we want to use the hook for,
> > > not generic changes that apply to all target.
> > >
> > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> > >
> > > Ok for master pending submission of Tejas' patch?
> >
> > I'm refraining from bike-shedding on the name, but ... "preferred"?
> >
>
> Understandable, my reasoning for using preferred is that because the
> hook runs first that other transformations may still transform it.
>
> I thought about this when I added the hook and the reason I made it
> run first is that it's easier to figure out what your input is by looking
> at the previous pass output, but also think if the result of the
> transformation results in something isel would have normally changed
> that we still want this to happen.
>
> Additionally I wanted to indicate that the hook can't do any legitimization
> and is really purely only for selection. But perhaps all of this is better in
> docs. So s/preferred/target?
>
Did you want me to rename this or just update the docs and submit Richi?
Thanks,
Tamar
> > > Thanks,
> > > Tamar
> > >
> > > gcc/ChangeLog:
> > >
> > > * target.def (preferred_instruction_selection): New.
> > > * doc/tm.texi.in: Document it.
> > > * doc/tm.texi: Regenerate
> > > * gimple-isel.cc (pass_gimple_isel::execute): Use it.
> > > * targhooks.cc (default_preferred_instruction_selection): New.
> > > * targhooks.h (default_preferred_instruction_selection): New.
> > >
> > > ---
> > > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> > > index
> >
> fd208f53844a157721dd8a0282f283da64cf5d93..35c1e2061f3bfed037a50
> > e01c8ff50c199801a1b 100644
> > > --- a/gcc/doc/tm.texi
> > > +++ b/gcc/doc/tm.texi
> > > @@ -6628,6 +6628,16 @@ like @code{cond_add@var{m}}. The default
> > implementation returns a zero
> > > constant of type @var{type}.
> > > @end deftypefn
> > >
> > > +@deftypefn {Target Hook} bool
> > TARGET_PREFERRED_INSTRUCTION_SELECTION (function *@var{fun},
> > gimple_stmt_iterator *@var{gsi})
> > > +This hook allows a target to give a preferred instruction selection for
> > > an
> > > +instruction or sequence of instructions before expand to allow expansion
> to
> > > +generate more efficient code.
> > > +
> > > +@var{fun} is the current function being considered and @var{gsi} is the
> > > +iterator pointing to the current instruction being optimized. The
> > > default
> > > +implementation does not do any rewriting and returns false.
> > > +@end deftypefn
> > > +
> >
> > The return value is not documented. As with your patch it would
> > indicate whether the CFG was changed, but I think as you name it
> > "preferred" the intent is to not do any further instruction selection
> > when the target did something?
> >
> > The current pass stmt iteration is a bit messy in this regard, in
> > particular also what allowed modifications of 'gsi' are concerned
> > which looks like some handlers might remove the current stmt
> > but we advance it anyway (causing to possibly skip a stmt).
> >
>
> Yeah I was wondering whether I need to document what the state the callees
> need to leave GSI in. You're right in that the contract is implicit..
>
> I'll add it.
>
> Thanks,
> Tamar
>
> > Apart from this documentation issue the change looks OK to me.
> >
> > Richard.
> >
> >
> > > @deftypefn {Target Hook} tree TARGET_GOACC_ADJUST_PRIVATE_DECL
> > (location_t @var{loc}, tree @var{var}, int @var{level})
> > > This hook, if defined, is used by accelerator target back-ends to adjust
> > > OpenACC variable declarations that should be made private to the given
> > > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> > > index
> >
> 14315dd508051037b7936c89638c05f07b6d3d6f..825599407379dadb2b1
> > 9e53f38a8af58ec3fca5b 100644
> > > --- a/gcc/doc/tm.texi.in
> > > +++ b/gcc/doc/tm.texi.in
> > > @@ -4352,6 +4352,8 @@ address; but often a machine-dependent
> > strategy can generate better code.
> > >
> > > @hook TARGET_PREFERRED_ELSE_VALUE
> > >
> > > +@hook TARGET_PREFERRED_INSTRUCTION_SELECTION
> > > +
> > > @hook TARGET_GOACC_ADJUST_PRIVATE_DECL
> > >
> > > @hook TARGET_GOACC_EXPAND_VAR_DECL
> > > diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc
> > > index
> >
> e745608904e3db77a88d861bc6512afe67a82480..ac1a4c56a66b608d963f
> > 93a2ce80549016fd0f1d 100644
> > > --- a/gcc/gimple-isel.cc
> > > +++ b/gcc/gimple-isel.cc
> > > @@ -1357,6 +1357,9 @@ pass_gimple_isel::execute (struct function
> *fun)
> > > {
> > > for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> > > {
> > > + /* Give the target first try at replacing the instruction. */
> > > + cfg_changed |= targetm.preferred_instruction_selection (fun, &gsi);
> > > +
> > > /* Pre-expand VEC_COND_EXPRs to .VCOND* internal function
> > > calls mapping to supported optabs. */
> > > gimple *g = gimple_expand_vec_cond_expr (&gsi);
> > > diff --git a/gcc/target.def b/gcc/target.def
> > > index
> >
> f288329ffcab81e2773c2066e60a470611f29e23..cdf95b8a8c871da18b60d
> > ec984be5bc2e8cea5a2 100644
> > > --- a/gcc/target.def
> > > +++ b/gcc/target.def
> > > @@ -2137,6 +2137,19 @@ constant of type @var{type}.",
> > > (unsigned ifn, tree type, unsigned nops, tree *ops),
> > > default_preferred_else_value)
> > >
> > > +DEFHOOK
> > > +(preferred_instruction_selection,
> > > + "This hook allows a target to give a preferred instruction selection for
> an\n\
> > > +instruction or sequence of instructions before expand to allow expansion
> > to\n\
> > > +generate more efficient code.\n\
> > > +\n\
> > > +@var{fun} is the current function being considered and @var{gsi} is
> the\n\
> > > +iterator pointing to the current instruction being optimized. The
> default\n\
> > > +implementation does not do any rewriting and returns false.",
> > > + bool,
> > > + (function *fun, gimple_stmt_iterator *gsi),
> > > + default_preferred_instruction_selection)
> > > +
> > > DEFHOOK
> > > (record_offload_symbol,
> > > "Used when offloaded functions are seen in the compilation unit and no
> > named\n\
> > > diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> > > index
> >
> 1873d572ba3fb59c4713e10b5c5b14afb029b953..c906a8055cc565f7ad86
> > fba878645aa6e1b61337 100644
> > > --- a/gcc/targhooks.cc
> > > +++ b/gcc/targhooks.cc
> > > @@ -2764,6 +2764,14 @@ default_preferred_else_value (unsigned, tree
> > type, unsigned, tree *)
> > > return build_zero_cst (type);
> > > }
> > >
> > > +/* The default implementation of
> > TARGET_PREFERRED_INSTRUCTION_SELECTION. */
> > > +
> > > +bool
> > > +default_preferred_instruction_selection (function *, gimple_stmt_iterator
> > *)
> > > +{
> > > + return false;
> > > +}
> > > +
> > > /* Default implementation of TARGET_HAVE_SPECULATION_SAFE_VALUE.
> > */
> > > bool
> > > default_have_speculation_safe_value (bool active ATTRIBUTE_UNUSED)
> > > diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> > > index
> >
> 92e7a4cb10f109978301d8c10f69b4c5a3952f56..4bfcbdd0c3012fdd67201
> > aa5da638eb8177e945e 100644
> > > --- a/gcc/targhooks.h
> > > +++ b/gcc/targhooks.h
> > > @@ -305,7 +305,8 @@ extern machine_mode
> > default_mode_for_floating_type (enum tree_index);
> > > extern HOST_WIDE_INT
> > default_stack_clash_protection_alloca_probe_range (void);
> > > extern void default_select_early_remat_modes (sbitmap);
> > > extern tree default_preferred_else_value (unsigned, tree, unsigned, tree
> *);
> > > -
> > > +extern bool default_preferred_instruction_selection (function *,
> > > + gimple_stmt_iterator *);
> > > extern bool default_have_speculation_safe_value (bool);
> > > extern bool speculation_safe_value_not_needed (bool);
> > > extern rtx default_speculation_safe_value (machine_mode, rtx, rtx, rtx);
> > >
> > >
> > >
> >
> > --
> > Richard Biener <[email protected]>
> > SUSE Software Solutions Germany GmbH,
> > Frankenstrasse 146, 90461 Nuernberg, Germany;
> > GF: Jochen Jaser, Andrew McDonald, Werner Knoblich; (HRB 36809, AG
> > Nuernberg)
