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? > > 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.
