On Sat, Aug 22, 2026 at 7:11 PM Jeffrey Law <[email protected]> wrote: > > > > On 8/15/2026 2:17 PM, Dominic P wrote: > > 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. > Thanks. I've pushed this to the trunk. > > One could legitimately ask if the caching is all that useful anyway. I > wouldn't be surprised if it was added when the synth_multiply code went > into GCC in the early 90s and this kind of caching would likely have > been seen as more important than it is today.
This was https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118072 . Where I even found out the change that introduced the issue was from 1998. I did mention maybe we should even reset this caching between bbs even. >From what I can tell this was done to try to catch the case of % and / being used in the same bb to be able to reuse the division already. > > Jeff
