[gcc r17-3551] [PATCH 2/3] expmed: do not carry last_div_const between functions
Jeff Law via Gcc-cvs <[email protected]>
| Newsgroups | gmane.comp.gcc.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://gcc.gnu.org/g:435528a4133bf61c4c25af0dca0ba2cd94facf79 commit r17-3551-g435528a4133bf61c4c25af0dca0ba2cd94facf79 Author: Dominic P <[email protected]> Date: Sat Aug 22 20:08:39 2026 -0600 [PATCH 2/3] expmed: do not carry last_div_const between functions expand_divmod remembers the constant of the last division it expanded so that a following modulo by the same constant can reuse the quotient. The state was a function-scope static that was never reset, so it carried from one function into the next and a function's code came to depend on what had been expanded before it in the translation unit. In the new testcase m1 and m2 are character for character identical, yet m1 inherits the state left by d and m2 inherits the state left by m1, so the two are expanded differently; reordering the functions in the source changes the generated code. Reset the state in prepare_function_start, which already does exactly this for another cache that expansion keeps in a different file: it resets bitint_extended, which is defined in expr.cc and declared in expr.h. Following that precedent keeps the reset on the one path that every function's expansion goes through, including the decl-less dummy pushed by init_dummy_function_start, and avoids having to key the state on the current function's identity. This only makes the existing heuristic deterministic. It is not a pessimisation of the modulo it changes: at -Os -mcpu=arm1176jzf-s the reused-quotient form of m1 is six instructions and the libcall form five, so the testcase's m1 shrinks by one instruction as it stops depending on d. Whether the heuristic should apply to a quotient that is merely the last one expanded, rather than one that is actually available for reuse at this point, is a separate question this patch does not touch. The series was bootstrapped on x86_64-pc-linux-gnu at trunk 7f549ea2b47 with the stage2/stage3 comparison successful, and a full make check shows no regressions: 227924 gcc and 278399 g++ expected passes, and every one of the 112 unexpected results also occurs with the series reverted. With the patch reverted and the test kept, the test emits one bl __aeabi_idivmod instead of two. Assisted-by: Claude Opus 5 (Anthropic) gcc/ChangeLog: * expmed.cc (last_div_const): New variable, moved out of... (expand_divmod): ...here. * expmed.h (last_div_const): Declare. * function.cc (prepare_function_start): Reset last_div_const. gcc/testsuite/ChangeLog: * gcc.target/arm/divmod-order-1.c: New test. Signed-off-by: Dominic P <[email protected]> Diff: --- gcc/expmed.cc | 8 +++++++- gcc/expmed.h | 4 ++++ gcc/function.cc | 4 ++++ gcc/testsuite/gcc.target/arm/divmod-order-1.c | 25 +++++++++++++++++++++++++ 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/gcc/expmed.cc b/gcc/expmed.cc index b87d06bc9a43..3cfe58d4d9b2 100644 --- a/gcc/expmed.cc +++ b/gcc/expmed.cc @@ -55,6 +55,13 @@ static bool store_integral_bit_field (rtx, opt_scalar_int_mode, unsigned HOST_WIDE_INT, poly_uint64, poly_uint64, machine_mode, rtx, bool, bool); +/* The constant divisor of the last division expanded, so that a modulo by + the same constant can reuse the quotient. Reset for each function by + prepare_function_start: carrying it across functions would make a + function's code depend on what was expanded before it in the translation + unit. */ +HOST_WIDE_INT last_div_const = 0; + static void store_fixed_bit_field (rtx, opt_scalar_int_mode, unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT, @@ -4370,7 +4377,6 @@ expand_divmod (int rem_flag, enum tree_code code, machine_mode mode, optab optab1, optab2; int op1_is_constant, op1_is_pow2 = 0; int max_cost, extra_cost; - static HOST_WIDE_INT last_div_const = 0; bool speed = optimize_insn_for_speed_p (); op1_is_constant = CONST_INT_P (op1); diff --git a/gcc/expmed.h b/gcc/expmed.h index 557b58392baf..78f3f36e6615 100644 --- a/gcc/expmed.h +++ b/gcc/expmed.h @@ -728,4 +728,8 @@ extern rtx expmed_mult_highpart_optab (scalar_int_mode, rtx, rtx, rtx, int, int); extern rtx expand_rotate_as_vec_perm (machine_mode, rtx, rtx, rtx); +/* The constant divisor of the last division expanded, reset per function + by prepare_function_start. */ +extern HOST_WIDE_INT last_div_const; + #endif // EXPMED_H diff --git a/gcc/function.cc b/gcc/function.cc index 5ade058d724c..9358787b262c 100644 --- a/gcc/function.cc +++ b/gcc/function.cc @@ -4978,6 +4978,10 @@ prepare_function_start (void) /* Reset the cache of the "extended" flag in the target's _BitInt info struct. */ bitint_extended = -1; + + /* Reset the divisor of the last division expanded, so that a function's + code does not depend on what was expanded before it. */ + last_div_const = 0; } void diff --git a/gcc/testsuite/gcc.target/arm/divmod-order-1.c b/gcc/testsuite/gcc.target/arm/divmod-order-1.c new file mode 100644 index 000000000000..eba68cff52a8 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/divmod-order-1.c @@ -0,0 +1,25 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target arm_arm_ok } */ +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-mthumb" } { "" } } */ +/* { dg-skip-if "do not override -mcpu" { *-*-* } { "-mcpu=*" } { "-mcpu=arm1176jzf-s" } } */ +/* { dg-skip-if "incompatible options" { arm*-*-* } { "-march=*" } { "" } } */ +/* { dg-options "-Os -marm -mcpu=arm1176jzf-s" } */ + +/* expand_divmod remembers the last division it expanded so that a following + modulo by the same constant can reuse the quotient. That state used to be a + file-static that was never reset, so it carried from one function into the + next: m1 and m2 below are character for character identical, yet m1 saw the + state left behind by d and m2 saw the state left behind by m1, and the two + compiled differently. Reordering the functions in the source changed the + generated code. + + Both must now be expanded the same way. */ + +int d (int x) { return x / 3; } +int m1 (int x) { return x % 3; } +int m2 (int x) { return x % 3; } + +/* m1 and m2 are identical, so they must make the same number of calls. d is a + plain division and is expanded inline, so every libcall here comes from the + two modulo functions: one each, never one and none. */ +/* { dg-final { scan-assembler-times "bl\\s+__aeabi_idivmod" 2 } } */