| Message ID | 20260903042855.25179-1-yqtian668@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ira: Ignore debug insns in simple-LRA threshold [PR108784] | expand |
On 9/3/26 12:28 AM, Yongqiang Tian wrote: > The simple-LRA decision uses get_max_uid() as an approximation of the > number of instructions. Debug instructions consume UIDs, so enabling > debug information can cross the threshold and change register allocation > and generated code. > > Use get_max_insn_count(), which excludes debug instructions, so the > decision depends only on ordinary instructions. Add an -fcompare-debug > regression test. > > Bootstrapped and regression tested on x86_64-pc-linux-gnu. > > PR debug/108784 > > gcc/ChangeLog: > > * ira.cc (ira): Use get_max_insn_count instead of get_max_uid when > selecting simple LRA. > > gcc/testsuite/ChangeLog: > > * gcc.dg/pr108784.c: New test. > > Assisted-by: OpenAI Codex 5.6 Sol > Assisted-by: Claude > Signed-off-by: Yongqiang Tian<yqtian668@gmail.com> > --- > gcc/ira.cc | 8 +++-- > gcc/testsuite/gcc.dg/pr108784.c | 63 +++++++++++++++++++++++++++++++++ > 2 files changed, 68 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/pr108784.c The patch complies with GCC AI policy. OK for me to commit it to trunk. Thank you for fixing this.
On 9/2/26 10:28 PM, Yongqiang Tian wrote: > The simple-LRA decision uses get_max_uid() as an approximation of the > number of instructions. Debug instructions consume UIDs, so enabling > debug information can cross the threshold and change register allocation > and generated code. > > Use get_max_insn_count(), which excludes debug instructions, so the > decision depends only on ordinary instructions. Add an -fcompare-debug > regression test. > > Bootstrapped and regression tested on x86_64-pc-linux-gnu. > > PR debug/108784 > > gcc/ChangeLog: > > * ira.cc (ira): Use get_max_insn_count instead of get_max_uid when > selecting simple LRA. > > gcc/testsuite/ChangeLog: > > * gcc.dg/pr108784.c: New test. > > Assisted-by: OpenAI Codex 5.6 Sol > Assisted-by: Claude > Signed-off-by: Yongqiang Tian<yqtian668@gmail.com> I don't recognize your email/name, so I'm going to assume no commit privileges at this time. So I went ahead and pushed this to the trunk given Vladimir's ack. These bugs are often hard to find, so thanks as ton for chasing it down! jeff
Hi Jeff and Vladimir, Thank you very much for reviewing and pushing it. I have found a few more bugs like this one, and I will report them after my Bugzilla account request is approved. Thank you and have a great day/night. Best, Yongqiang On Sat, 5 Sept 2026 at 02:59, Jeff Law <jeffrey.law@oss.qualcomm.com> wrote: > > > > On 9/2/26 10:28 PM, Yongqiang Tian wrote: > > The simple-LRA decision uses get_max_uid() as an approximation of the > > number of instructions. Debug instructions consume UIDs, so enabling > > debug information can cross the threshold and change register allocation > > and generated code. > > > > Use get_max_insn_count(), which excludes debug instructions, so the > > decision depends only on ordinary instructions. Add an -fcompare-debug > > regression test. > > > > Bootstrapped and regression tested on x86_64-pc-linux-gnu. > > > > PR debug/108784 > > > > gcc/ChangeLog: > > > > * ira.cc (ira): Use get_max_insn_count instead of get_max_uid when > > selecting simple LRA. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.dg/pr108784.c: New test. > > > > Assisted-by: OpenAI Codex 5.6 Sol > > Assisted-by: Claude > > Signed-off-by: Yongqiang Tian<yqtian668@gmail.com> > I don't recognize your email/name, so I'm going to assume no commit > privileges at this time. So I went ahead and pushed this to the trunk > given Vladimir's ack. > > These bugs are often hard to find, so thanks as ton for chasing it down! > > jeff
diff --git a/gcc/ira.cc b/gcc/ira.cc index 798a7f01c5d..dd8c64f5dcb 100644 --- a/gcc/ira.cc +++ b/gcc/ira.cc @@ -5765,9 +5765,11 @@ ira (FILE *f) lra_simple_p = (ira_use_lra_p && (num_used_regs >= (1U << 26) / last_basic_block_for_fn (cfun) - /* max uid is a good evaluation of the number of insns as most - optimizations are done on tree-SSA level. */ - || ((uint64_t) get_max_uid () + /* get_max_insn_count is a good evaluation of the number of insns + as most optimizations are done on tree-SSA level. Unlike + get_max_uid, it excludes debug insns, so the choice below does + not depend on debug information. */ + || ((uint64_t) get_max_insn_count () > (uint64_t) param_ira_simple_lra_insn_threshold * 1000))); if (lra_simple_p) diff --git a/gcc/testsuite/gcc.dg/pr108784.c b/gcc/testsuite/gcc.dg/pr108784.c new file mode 100644 index 00000000000..6a8eff6c3dd --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr108784.c @@ -0,0 +1,63 @@ +/* PR debug/108784 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -w --param=ira-simple-lra-insn-threshold=1 -fcompare-debug" } */ + +extern unsigned external_mix (unsigned); + +/* These unused named locals generate debug insns without affecting code. */ +#define GHOST(P, I, N) unsigned ghost##P##I = x ^ (N) ^ (I) +#define GHOSTS_8(P, N) \ + GHOST (P, 0, N); GHOST (P, 1, N); GHOST (P, 2, N); GHOST (P, 3, N); \ + GHOST (P, 4, N); GHOST (P, 5, N); GHOST (P, 6, N); GHOST (P, 7, N) +#define GHOSTS_64(N) \ + GHOSTS_8 (a, N); GHOSTS_8 (b, N); GHOSTS_8 (c, N); GHOSTS_8 (d, N); \ + GHOSTS_8 (e, N); GHOSTS_8 (f, N); GHOSTS_8 (g, N); GHOSTS_8 (h, N) + +#define STEP(N, A, I, C) \ + do \ + { \ + GHOSTS_64 (N); \ + A = A * 33u + x + p[I]; \ + x = x * 1103515245u + A + (C); \ + __asm__ volatile ("" : "+r" (x)); \ + } \ + while (0) + +#define UNIT() \ + STEP (1u, a0, 0, 12345u); \ + STEP (2u, a1, 1, 12346u); \ + STEP (3u, a2, 2, 12347u); \ + STEP (4u, a3, 3, 12348u); \ + STEP (5u, a4, 4, 12349u); \ + STEP (6u, a5, 5, 12350u); \ + STEP (7u, a6, 6, 12351u); \ + STEP (8u, a7, 7, 12352u); \ + STEP (9u, a8, 0, 12353u); \ + STEP (10u, a9, 1, 12354u); \ + STEP (11u, a10, 2, 12355u); \ + STEP (12u, a11, 3, 12356u); \ + if ((x & 31u) == 7u) x ^= external_mix (x); + +#define REP_1() UNIT() +#define REP_2() REP_1() REP_1() + +__attribute__((noinline)) +unsigned +foo (unsigned x, const unsigned *p) +{ + unsigned a0 = x + p[0] + 1u; + unsigned a1 = x + p[1] + 2u; + unsigned a2 = x + p[2] + 3u; + unsigned a3 = x + p[3] + 4u; + unsigned a4 = x + p[4] + 5u; + unsigned a5 = x + p[5] + 6u; + unsigned a6 = x + p[6] + 7u; + unsigned a7 = x + p[7] + 8u; + unsigned a8 = x + p[0] + 9u; + unsigned a9 = x + p[1] + 10u; + unsigned a10 = x + p[2] + 11u; + unsigned a11 = x + p[3] + 12u; + REP_2 (); + return x + a0 + a1 + a2 + a3 + a4 + a5 + a6 + a7 + a8 + a9 + a10 + + a11; +}
The simple-LRA decision uses get_max_uid() as an approximation of the number of instructions. Debug instructions consume UIDs, so enabling debug information can cross the threshold and change register allocation and generated code. Use get_max_insn_count(), which excludes debug instructions, so the decision depends only on ordinary instructions. Add an -fcompare-debug regression test. Bootstrapped and regression tested on x86_64-pc-linux-gnu. PR debug/108784 gcc/ChangeLog: * ira.cc (ira): Use get_max_insn_count instead of get_max_uid when selecting simple LRA. gcc/testsuite/ChangeLog: * gcc.dg/pr108784.c: New test. Assisted-by: OpenAI Codex 5.6 Sol Assisted-by: Claude Signed-off-by: Yongqiang Tian <yqtian668@gmail.com> --- gcc/ira.cc | 8 +++-- gcc/testsuite/gcc.dg/pr108784.c | 63 +++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/pr108784.c