On Tue, 18 Nov 2025, Konstantinos Eleftheriou wrote:
> On Mon, Nov 17, 2025 at 2:51 PM Richard Biener <[email protected]> wrote:
>
> > On Mon, 17 Nov 2025, Konstantinos Eleftheriou wrote:
> >
> > > Hi Richard, thanks for the feedback.
> > >
> > > On Thu, Nov 13, 2025 at 12:07 PM Richard Biener <[email protected]>
> > wrote:
> > >
> > > > On Wed, 12 Nov 2025, Konstantinos Eleftheriou wrote:
> > > >
> > > > > This patch adds a new cost function so that the targets can decide
> > if the
> > > > > store/load reordering will be profitable. It also adds the
> > > > > `asf-default-cost-value` parameter, which is used as the default
> > return
> > > > > value for the cost function.
> > > >
> > > > It's a bit odd the param has "cost_value" and the hook says cost_p when
> > > > this is a simple flag in the end. To that extent the "default" hook
> > > > should be either returning true or false and the knob the user has
> > > > is -f[no-]avoid-store-forwarding already, no need to add a --param.
> > > >
> > > The new parameter is "false" by default and "true" for AArch64 only. In
> > > case that we keep the default implementation, if we change it to `return
> > > flag_avoid_store_forwarding` and remove the new parameter, it will return
> > > "true" for all targets, as the pass will be enabled by default on all
> > > targets. Is that okay?
> >
> > I don't see why we need to have a target hook then. I think it makes
> > sense that target can opt out of the added compile-time of the whole
> > pass if it does not implement any store-to-load forwarding (AVR?),
> > so I'd like to see this implemented in terms of that.
> >
> > That said, we have other passes (ccmp?) that face a similar situation
> > and I think we should have some consistency here. But I do not remember
> > whether we settled on a canonical way to gate passes based on target
> > features.
> >
> We are thinking of adding a flag that would be set to "true" on targets
> that
> want to use the pass and enable it by default on AArch64 and maybe x86-64.
> How does that sound?
Well. I think it's OK to enable on a subset of targets by default,
but it should be target maintainers say whether a target is among the
crowd or not. There's the target specific optimization_table as
mechanism to achieve such defaults, like
gcc/common/config/i386/i386-common.cc
static const struct default_options ix86_option_optimization_table[] =
{
/* Enable redundant extension instructions removal at -O2 and higher.
*/
{ OPT_LEVELS_2_PLUS, OPT_free, NULL, 1 },
/* Enable function splitting at -O2 and higher. */
{ OPT_LEVELS_2_PLUS, OPT_freorder_blocks_and_partition, NULL, 1 },
...
that's the canonical way to achieve a per-target default.
Iff there's an actual mechanism that a target can do individual
costing that might be also a way to signal support and we can
decide a default based on such implementation, but I don't see that
here so I suggest to go the above way.
I've seen no agreement to enable this on x86 btw.
Richard.
> Konstantinos.
>
> >
> > Richard.
> >
> > > >
> > > > One could even say the target hook should _not_ have a default
> > > > implementation and only targets that implement the hook should have
> > > > the pass enabled - we only pay with compile-time cost otherwise.
> > > >
> > > > So please rework this part.
> > > >
> > > > Richard.
> > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > > * doc/tm.texi: Regenerate.
> > > > > * doc/tm.texi.in: Document new target hook.
> > > > > * params.opt: New parameter.
> > > > > * target.def: New DEFHOOK.
> > > > > * targhooks.cc (default_avoid_store_forwarding_reorder_cost_p):
> > > > > New function.
> > > > > * targhooks.h (default_avoid_store_forwarding_reorder_cost_p):
> > > > > New function declaration.
> > > > >
> > > > > Signed-off-by: Konstantinos Eleftheriou <
> > > > [email protected]>
> > > > > ---
> > > > >
> > > > > (no changes since v1)
> > > > >
> > > > > gcc/doc/tm.texi | 7 +++++++
> > > > > gcc/doc/tm.texi.in | 2 ++
> > > > > gcc/params.opt | 4 ++++
> > > > > gcc/target.def | 9 +++++++++
> > > > > gcc/targhooks.cc | 9 +++++++++
> > > > > gcc/targhooks.h | 5 +++++
> > > > > 6 files changed, 36 insertions(+)
> > > > >
> > > > > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> > > > > index fd208f53844a..aef552aac5f8 100644
> > > > > --- a/gcc/doc/tm.texi
> > > > > +++ b/gcc/doc/tm.texi
> > > > > @@ -7466,6 +7466,13 @@ needed, the sequence cost and additional
> > relevant
> > > > information is given in
> > > > > the arguments so that the target can make an informed decision.
> > > > > @end deftypefn
> > > > >
> > > > > +@deftypefn {Target Hook} bool
> > > > TARGET_AVOID_STORE_FORWARDING_REORDER_COST_P (machine_mode,
> > > > @var{machine_mode}, @var{HOST_WIDE_INT})
> > > > > +This hook decides if it's profitable for the target to rearrange a
> > store
> > > > > +and a load instruction to avoid a potential store forwarding stall.
> > > > > +The store and load modes and the byte offset of the store within the
> > > > > +load are given as arguments.
> > > > > +@end deftypefn
> > > > > +
> > > > > @node Scheduling
> > > > > @section Adjusting the Instruction Scheduler
> > > > >
> > > > > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> > > > > index 14315dd50805..0c3168cb6dde 100644
> > > > > --- a/gcc/doc/tm.texi.in
> > > > > +++ b/gcc/doc/tm.texi.in
> > > > > @@ -4781,6 +4781,8 @@ Define this macro if a non-short-circuit
> > operation
> > > > produced by
> > > > >
> > > > > @hook TARGET_AVOID_STORE_FORWARDING_P
> > > > >
> > > > > +@hook TARGET_AVOID_STORE_FORWARDING_REORDER_COST_P
> > > > > +
> > > > > @node Scheduling
> > > > > @section Adjusting the Instruction Scheduler
> > > > >
> > > > > diff --git a/gcc/params.opt b/gcc/params.opt
> > > > > index f8884e976e7f..96720651731d 100644
> > > > > --- a/gcc/params.opt
> > > > > +++ b/gcc/params.opt
> > > > > @@ -1091,6 +1091,10 @@ Maximum size of a single store merging region
> > in
> > > > bytes.
> > > > > Common Joined UInteger Var(param_store_forwarding_max_distance)
> > > > Init(10) IntegerRange(0, 1000) Param Optimization
> > > > > Maximum number of instruction distance that a small store forwarded
> > to
> > > > a larger load may stall. Value '0' disables the cost checks for the
> > > > avoid-store-forwarding pass.
> > > > >
> > > > > +-param=asf-default-cost-value=
> > > > > +Common Joined UInteger Var(param_asf_default_cost_value) Init(0)
> > > > IntegerRange(0, 1) Param Optimization
> > > > > +Default return value for the avoid_store_forwarding_reorder_cost_p
> > > > function.
> > > > > +
> > > > > -param=switch-conversion-max-branch-ratio=
> > > > > Common Joined UInteger Var(param_switch_conversion_branch_ratio)
> > > > Init(8) IntegerRange(1, 65536) Param Optimization
> > > > > The maximum ratio between array size and switch branches for a
> > switch
> > > > conversion to take place.
> > > > > diff --git a/gcc/target.def b/gcc/target.def
> > > > > index f288329ffcab..d60510e5458e 100644
> > > > > --- a/gcc/target.def
> > > > > +++ b/gcc/target.def
> > > > > @@ -7158,6 +7158,15 @@ the arguments so that the target can make an
> > > > informed decision.",
> > > > > bool, (vec<store_fwd_info>, rtx, int, bool),
> > > > > default_avoid_store_forwarding_p)
> > > > >
> > > > > +DEFHOOK
> > > > > +(avoid_store_forwarding_reorder_cost_p,
> > > > > + "This hook decides if it's profitable for the target to rearrange a
> > > > store\n\
> > > > > +and a load instruction to avoid a potential store forwarding
> > stall.\n\
> > > > > +The store and load modes and the byte offset of the store within
> > the\n\
> > > > > +load are given as arguments.",
> > > > > + bool, (machine_mode, machine_mode, HOST_WIDE_INT),
> > > > > + default_avoid_store_forwarding_reorder_cost_p)
> > > > > +
> > > > > /* Determine the type of unwind info to emit for debugging. */
> > > > > DEFHOOK
> > > > > (debug_unwind_info,
> > > > > diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> > > > > index 1873d572ba3f..15d882f4703e 100644
> > > > > --- a/gcc/targhooks.cc
> > > > > +++ b/gcc/targhooks.cc
> > > > > @@ -2352,6 +2352,15 @@ default_avoid_store_forwarding_p
> > > > (vec<store_fwd_info>, rtx, int total_cost,
> > > > > return true;
> > > > > }
> > > > >
> > > > > +/* The default implementation of
> > TARGET_AVOID_STORE_FORWARDING_COST_P.
> > > > */
> > > > > +
> > > > > +bool
> > > > > +default_avoid_store_forwarding_reorder_cost_p (machine_mode,
> > > > machine_mode,
> > > > > + HOST_WIDE_INT)
> > > > > +{
> > > > > + return param_asf_default_cost_value;
> > > > > +}
> > > > > +
> > > > > /* Determine the debugging unwind mechanism for the target. */
> > > > >
> > > > > enum unwind_info_type
> > > > > diff --git a/gcc/targhooks.h b/gcc/targhooks.h
> > > > > index 92e7a4cb10f1..5389ea1cb78b 100644
> > > > > --- a/gcc/targhooks.h
> > > > > +++ b/gcc/targhooks.h
> > > > > @@ -264,6 +264,11 @@ extern unsigned char default_class_max_nregs
> > > > (reg_class_t, machine_mode);
> > > > > extern bool default_avoid_store_forwarding_p (vec<store_fwd_info>,
> > rtx,
> > > > int,
> > > > > bool);
> > > > >
> > > > > +extern bool
> > > > > +default_avoid_store_forwarding_reorder_cost_p (machine_mode
> > store_mode,
> > > > > + machine_mode load_mode,
> > > > > + HOST_WIDE_INT offset);
> > > > > +
> > > > > extern enum unwind_info_type default_debug_unwind_info (void);
> > > > >
> > > > > extern void default_canonicalize_comparison (int *, rtx *, rtx *,
> > bool);
> > > > >
> > > >
> > > > --
> > > > 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)
> > > >
> > > Konstantinos.
> > >
> >
> > --
> > 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)
