[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 } } */
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.