On Tue, 18 Nov 2025, Tamar Christina wrote:
> > -----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?
Update the docs with respect to the gsi constraints and with respect
to the return value semantics (and whether the target is allowed to
"change the CFG"). I'd shorten the name to TARGET_INSTRUCTION_SELECTION
as well.
Richard.
> 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)
>
--
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)
