Hi Maxim, Thank you for the review and for pointing this out. The current patch combines two separate changes, which is not sufficiently clear from the description. The -msched-weight heuristic is unrelated to negative offsets; those are prevented by the unconditional DONT_BREAK_DEPENDENCIES, which stops the scheduler from moving a base-register update across a load or store and compensating by changing the address offset. I will split these changes into separate patches in the next revision.
I will also compare the new heuristic with both existing -fsched-pressure algorithms and collect benchmark data. Based on those results, I will either adapt the existing register-pressure-aware scheduling for P5600 or provide a clearer justification for keeping a target-specific implementation. Thanks again, Eldar. On 8/13/26 23:20, Maxim Kuvyrkov wrote: > [You don't often get email from [email protected]. Learn why this is > important at https://aka.ms/LearnAboutSenderIdentification ] > > CAUTION: This email originated from outside of the organization. Do not click > links or open attachments unless you recognize the sender and know the > content is safe. > > > 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
