Re: [PATCH v3 6/6] MIPS: P5600: Option -msched-weight added

Hi Eldar,

> On Jun 19, 2026, at 03:39, Eldar Osmanovic <[email protected]> wrote:
> 
> From: Jaydeep Patil <[email protected]>
> 
> Fix negative offset memory addressing. Unconditionally set
> DONT_BREAK_DEPENDENCIES in scheduling flags. The code to break
> dependencies does not appear to provide a win under any circumstance and
> is often harmful. Disable it completely pending further investigation.
> 

How does this patch fix negative offset memory addressing?

From what I understand, the patch implements yet another approach to add register pressure sensitivity to the sched1 pass.  The pass calculates instruction "weight" based on number of register births-deaths, and then pushes the scheduler to prioritize insns with highest (or lowest?) weight.  How does this transfer into improvements for negative offset memory addressing?

Also, why not adapt existing register-pressure aware scheduling for this?

Finally, do you benchmark data for before and after the patch?

Thanks!

--
Maxim Kuvyrkov
Garden City Compilers


> gcc/
> * config/mips/mips.cc (level, consumer_luid): New static global
> variables.
> (LEVEL, CONSUMER_LUID): New macros.
> (find_reg_born): New static function.
> (get_weight): Likewise.
> (mips_weight_init_global): Likewise.
> (mips_sched_init_global): Likewise.
> (mips_weight_evaluation): Likewise.
> (mips_evaluation_hook): Likewise.
> (mips_set_sched_flags): Likewise. Fix negative offset memory
> addressing. Unconditionally set DONT_BREAK_DEPENDENCIES in
> scheduling flags. The code to break dependencies does not appear
> to provide a win under any circumstance and is often harmful.
> Disable it completely pending further investigation.
> (mips_weight_finish_global): New static function.
> (mips_sched_finish_global): Likewise.
> (mips_sched_weight): Likewise.
> (mips_sched_reorder_1): Call mips_sched_weight.
> (TARGET_SCHED_INIT_GLOBAL, TARGET_SCHED_FINISH_GLOBAL,
> TARGET_SCHED_DEPENDENCIES_EVALUATION_HOOK,
> TARGET_SCHED_SET_SCHED_FLAGS): New macros.
> * config/mips/mips.opt (-msched-weight): New option.
> 
> gcc/testsuite/
> * gcc.target/mips/mips.exp: Add sched-weight to the test options.
> * gcc.target/mips/sched-weight-1.c: New test.
> 
> Cherry-picked 0cf2542b41d8102800af180f0b6da1fe55a9d76b,
> and f732af3ad1a393d2f2e708f0d7c469a093049d01
> from https://github.com/MIPS/gcc
> 
> Signed-off-by: Matthew Fortune <[email protected]>
> Signed-off-by: Prachi Godbole <[email protected]>
> Signed-off-by: Jaydeep Patil <[email protected]>
> Signed-off-by: Faraz Shahbazker <[email protected]>
> Signed-off-by: Aleksandar Rakic <[email protected]>
> Signed-off-by: Eldar Osmanovic <[email protected]>
> ---
> gcc/config/mips/mips.cc                       | 241 ++++++++++++++++++
> gcc/config/mips/mips.opt                      |   3 +
> gcc/testsuite/gcc.target/mips/mips.exp        |   1 +
> .../gcc.target/mips/sched-weight-1.c          |  21 ++
> 4 files changed, 266 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/mips/sched-weight-1.c
> 
> diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
> index 2a70ab98a26..cc0be1bb014 100644
> --- a/gcc/config/mips/mips.cc
> +++ b/gcc/config/mips/mips.cc
> @@ -73,6 +73,17 @@ along with GCC; see the file COPYING3.  If not see
> /* This file should be included last.  */
> #include "target-def.h"
> 
> +/* Definitions used in ready queue reordering for first scheduling pass.  */
> +
> +static int *level = NULL;
> +static int *consumer_luid = NULL;
> +
> +#define LEVEL(INSN) \
> +  level[INSN_UID ((INSN))]
> +
> +#define CONSUMER_LUID(INSN) \
> +  consumer_luid[INSN_UID ((INSN))]
> +
> /* True if X is an UNSPEC wrapper around a SYMBOL_REF or LABEL_REF.  */
> #define UNSPEC_ADDRESS_P(X) \
>   (GET_CODE (X) == UNSPEC \
> @@ -15562,6 +15573,218 @@ mips_74k_agen_reorder (rtx_insn **ready, int nready)
>     }
> }
> 
> +
> +/* These functions are called when -msched-weight is set.  */
> +
> +/* Find register born in given X if any.  */
> +
> +static int
> +find_reg_born (rtx x)
> +{
> +  if (GET_CODE (x) == CLOBBER)
> +    return 1;
> +
> +  if (GET_CODE (x) == SET)
> +    {
> +      if (REG_P (SET_DEST (x)) && reg_mentioned_p (SET_DEST (x), SET_SRC (x)))
> + return 0;
> +      return 1;
> +    }
> +  return 0;
> +}
> +
> +/* Calculate register weight for given INSN.  */
> +
> +static int
> +get_weight (rtx insn)
> +{
> +  int weight = 0;
> +  rtx x;
> +
> +  /* Increment weight for each register born here.  */
> +  x = PATTERN (insn);
> +  weight = find_reg_born (x);
> +
> +  if (GET_CODE (x) == PARALLEL)
> +    {
> +      int i;
> +      for (i = XVECLEN (x, 0) - 1; i >= 0; i--)
> + {
> +  x = XVECEXP (PATTERN (insn), 0, i);
> +  weight += find_reg_born (x);
> + }
> +    }
> +
> +  /* Decrement weight for each register that dies here.  */
> +  for (x = REG_NOTES (insn); x; x = XEXP (x, 1))
> +    {
> +      if (REG_NOTE_KIND (x) == REG_DEAD || REG_NOTE_KIND (x) == REG_UNUSED)
> + {
> +  rtx note = XEXP (x, 0);
> +  if (REG_P (note))
> +    weight--;
> + }
> +    }
> +  return weight;
> +}
> +
> +/* TARGET_SCHED_WEIGHT helper function.
> +   Allocate and initialize global data.  */
> +
> +static void
> +mips_weight_init_global (int old_max_uid)
> +{
> +  level = (int *) xcalloc (old_max_uid, sizeof (int));
> +  consumer_luid = (int *) xcalloc (old_max_uid, sizeof (int));
> +}
> +
> +/* Implement TARGET_SCHED_INIT_GLOBAL.  */
> +
> +static void
> +mips_sched_init_global (FILE *dump ATTRIBUTE_UNUSED,
> + int verbose ATTRIBUTE_UNUSED,
> + int old_max_uid)
> +{
> +  if (!reload_completed && TARGET_SCHED_WEIGHT)
> +    mips_weight_init_global (old_max_uid);
> +}
> +
> +/* TARGET_SCHED_WEIGHT helper function.  Called for each basic block
> +   with dependency chain information in HEAD and TAIL.
> +   Calculates LEVEL for each INSN from its forward dependencies
> +   and finds out UID of first consumer instruction (CONSUMER_LUID) of INSN.  */
> +
> +static void
> +mips_weight_evaluation (rtx_insn *head, rtx_insn *tail)
> +{
> +  sd_iterator_def sd_it;
> +  dep_t dep;
> +  rtx_insn *prev_head, *insn;
> +  rtx x;
> +  prev_head = PREV_INSN (head);
> +
> +  for (insn = tail; insn != prev_head; insn = PREV_INSN (insn))
> +    if (INSN_P (insn))
> +      {
> + FOR_EACH_DEP (insn, SD_LIST_FORW, sd_it, dep)
> +  {
> +    x = DEP_CON (dep);
> +    if (! DEBUG_INSN_P (x))
> +      {
> + if (LEVEL (x) > LEVEL (insn))
> +  LEVEL (insn) = LEVEL (x);
> + CONSUMER_LUID (insn) = INSN_LUID (x);
> +      }
> +  }
> + LEVEL (insn)++;
> +      }
> +}
> +
> +/* Implement TARGET_SCHED_DEPENDENCIES_EVALUATION_HOOK.  */
> +
> +static void
> +mips_evaluation_hook (rtx_insn *head, rtx_insn *tail)
> +{
> +  if (!reload_completed && TARGET_SCHED_WEIGHT)
> +    mips_weight_evaluation (head, tail);
> +}
> +
> +/* Implement TARGET_SCHED_SET_SCHED_FLAGS.
> +   Enables DONT_BREAK_DEPENDENCIES for the first scheduling pass.
> +   It prevents breaking of dependencies on mem/inc pair in the first pass
> +   which would otherwise increase stalls.  */
> +
> +static void
> +mips_set_sched_flags (spec_info_t spec_info ATTRIBUTE_UNUSED)
> +{
> +  unsigned int *flags = &(current_sched_info->flags);
> +  *flags |= DONT_BREAK_DEPENDENCIES;
> +}
> +
> +static void
> +mips_weight_finish_global ()
> +{
> +  if (level != NULL)
> +    free (level);
> +
> +  if (consumer_luid != NULL)
> +    free (consumer_luid);
> +}
> +
> +/* Implement TARGET_SCHED_FINISH_GLOBAL.  */
> +
> +static void
> +mips_sched_finish_global (FILE *dump ATTRIBUTE_UNUSED,
> +  int verbose ATTRIBUTE_UNUSED)
> +{
> +  if (!reload_completed && TARGET_SCHED_WEIGHT)
> +    mips_weight_finish_global ();
> +}
> +
> +
> +/* This is a TARGET_SCHED_WEIGHT (option -msched-weight) helper function
> +   which is called during reordering of instructions in the first pass
> +   of the scheduler.  The function swaps the instruction at (NREADY - 1)
> +   of the READY list with another instruction in READY list as per
> +   the following algorithm.  The scheduler then picks the instruction
> +   at READY[NREADY - 1] and schedules it.
> +
> +   Every instruction is assigned with a value LEVEL.
> +   [See: mips_weight_evaluation ().]
> +
> +   1. INSN with highest LEVEL is chosen to be scheduled next, ties broken by
> +      1a. Choosing INSN that is used early in the flow or
> +      1b. Choosing INSN with greater INSN_TICK.
> +
> +   2. Choose INSN having less LEVEL number iff,
> +      2a. It is used early and
> +      2b. Has greater INSN_TICK and
> +      2c. Contributes less to the register pressure.  */
> +
> +static void
> +mips_sched_weight (rtx_insn **ready, int nready)
> +{
> +  int max_level = LEVEL (ready[nready-1]), toswap = nready-1;
> +  int i;
> +#define INSN_TICK(INSN) (HID (INSN)->tick)
> +
> +  for (i = nready - 2; i >= 0; i--)
> +    {
> +      rtx_insn *insn = ready[i];
> +      if (LEVEL (insn) == max_level)
> + {
> +  if (INSN_PRIORITY (insn) >= INSN_PRIORITY (ready[toswap]))
> +    {
> +      if (CONSUMER_LUID (insn) < CONSUMER_LUID (ready[toswap]))
> + toswap = i;
> +    }
> +  else if (INSN_TICK (insn) > INSN_TICK (ready[toswap]))
> +    toswap = i;
> + }
> +      if (LEVEL (insn) > max_level)
> + {
> +  max_level = LEVEL (insn);
> +  toswap = i;
> + }
> +      if (LEVEL (insn) < max_level)
> + {
> +  if (CONSUMER_LUID (insn) < CONSUMER_LUID (ready[toswap])
> +      && INSN_TICK (insn) > INSN_TICK (ready[toswap])
> +      && get_weight (insn) < get_weight (ready[toswap]))
> +    toswap = i;
> + }
> +    }
> +
> +  if (toswap != (nready-1))
> +    {
> +      rtx_insn *temp = ready[nready-1];
> +      ready[nready-1] = ready[toswap];
> +      ready[toswap] = temp;
> +    }
> +#undef INSN_TICK
> +}
> +
> +
> /* Implement TARGET_SCHED_INIT.  */
> 
> static void
> @@ -15598,6 +15821,11 @@ mips_sched_reorder_1 (FILE *file ATTRIBUTE_UNUSED, int verbose ATTRIBUTE_UNUSED,
> 
>   if (TUNE_74K)
>     mips_74k_agen_reorder (ready, *nreadyp);
> +
> +  if (! reload_completed
> +      && TARGET_SCHED_WEIGHT
> +      && *nreadyp > 1)
> +    mips_sched_weight (ready, *nreadyp);
> }
> 
> /* Implement TARGET_SCHED_REORDER.  */
> @@ -24063,6 +24291,19 @@ mips_print_patchable_function_entry (FILE *file ATTRIBUTE_UNUSED,
> #undef TARGET_C_MODE_FOR_FLOATING_TYPE
> #define TARGET_C_MODE_FOR_FLOATING_TYPE mips_c_mode_for_floating_type
> 
> +#undef TARGET_SCHED_INIT_GLOBAL
> +#define TARGET_SCHED_INIT_GLOBAL mips_sched_init_global
> +
> +#undef TARGET_SCHED_FINISH_GLOBAL
> +#define TARGET_SCHED_FINISH_GLOBAL mips_sched_finish_global
> +
> +#undef TARGET_SCHED_DEPENDENCIES_EVALUATION_HOOK
> +#define TARGET_SCHED_DEPENDENCIES_EVALUATION_HOOK mips_evaluation_hook
> +
> +#undef TARGET_SCHED_SET_SCHED_FLAGS
> +#define TARGET_SCHED_SET_SCHED_FLAGS mips_set_sched_flags
> +
> +
> #undef TARGET_DOCUMENTATION_NAME
> #define TARGET_DOCUMENTATION_NAME "MIPS"
> 
> diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
> index ad879176c39..133d8031246 100644
> --- a/gcc/config/mips/mips.opt
> +++ b/gcc/config/mips/mips.opt
> @@ -516,3 +516,6 @@ Use Loongson EXTension (EXT) instructions.
> mloongson-ext2
> Target Var(TARGET_LOONGSON_EXT2)
> Use Loongson EXTension R2 (EXT2) instructions.
> +
> +msched-weight
> +Target Var(TARGET_SCHED_WEIGHT) Undocumented
> diff --git a/gcc/testsuite/gcc.target/mips/mips.exp b/gcc/testsuite/gcc.target/mips/mips.exp
> index eba34ebf6a7..5b760125419 100644
> --- a/gcc/testsuite/gcc.target/mips/mips.exp
> +++ b/gcc/testsuite/gcc.target/mips/mips.exp
> @@ -300,6 +300,7 @@ foreach option {
>     relax-pic-calls
>     mcount-ra-address
>     odd-spreg
> +    sched-weight
>     msa
>     loongson-mmi
>     loongson-ext
> diff --git a/gcc/testsuite/gcc.target/mips/sched-weight-1.c b/gcc/testsuite/gcc.target/mips/sched-weight-1.c
> new file mode 100644
> index 00000000000..90be360d1c9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/mips/sched-weight-1.c
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "isa=p5600 -mtune=p5600 -mgp32 -mno-mips16 -mno-micromips -msched-weight -fdump-rtl-sched1" } */
> +/* { dg-skip-if "requires -O2" { *-*-* } { "*" } { "-O2" } } */
> +/* { dg-skip-if "requires non-LTO" { *-*-* } { "-flto" } { "" } } */
> +
> +int
> +foo (int *p, int a, int b, int c, int d)
> +{
> +  int x0 = p[0] + a;
> +  int x1 = p[1] + b;
> +  int x2 = p[2] + c;
> +  int x3 = p[3] + d;
> +  int x4 = p[4] + a;
> +  int x5 = p[5] + b;
> +  int y0 = x0 * x3;
> +  int y1 = x1 * x4;
> +  int y2 = x2 * x5;
> +  return y0 + y1 + y2;
> +}
> +
> +/* { dg-final { scan-rtl-dump {\[ x0_[^\n]*\n.*\(set \(reg:SI [0-9]+ \[ MEM\[\(int \*\)p_[^\n]* \+ 8B\] \]} "sched1" } } */
> -- 
> 2.43.0