On Thu, 8 Oct 2026, Julius Bairaktaris wrote: > On Thu, 8 Oct 2026, Richard Biener wrote: > > Hmm, I think this issue might be more wide-spread and using arbitrary > > RTL APIs during RTL expansion is now dangerous. That said, > > mips16_gp_pseudo_reg should probably not do this, since try-expansion > > and throw away is expected to leave no trail. > > > > How do we get there? > > Hi Richard, > > this is my first submission to GCC, so please excuse me if I get > something wrong. After reading through the code myself and discussing > it with Claude, my understanding is that we get there from a call > during expansion: > > expand_call -> mips_expand_call -> mips_load_call_address > -> mips_got_load -> mips_pic_base_register -> mips16_gp_pseudo_reg > > mips16_gp_pseudo_reg does push_topmost_sequence, walks get_insns() to > the first real insn and emits the $gp load after it. In the testcase > that is the basic block note of bb 2, which is already expanded but > not BB_RTL yet.
Wow, OK. So we have push_topmost_sequence, never heard of that. And it's used in few places like __builtin_apply_args expansion and in some targets. So the issue does look more prevalent. But! We're dealing with sequences here, not with "basic-blocks", so mips should probably be using emit_insn_before instead of add_insn_before? > I agree with your hint that this is better fixed in mips.cc. The load > could be put on the entry edge instead: > > start_sequence (); > ... emit the $gp load and the clobbers ... > insert_insn_on_edge (end_sequence (), > single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun))); > > pass_expand commits edge insertions after all blocks are expanded, so > nothing is emitted into the insn stream during expansion. > > Should I send a v2 like that instead of the emit-rtl.cc change? > > Julius > > On Thu, Oct 08, 2026 08:42 AM, Richard Biener <[email protected]> wrote: > > > > On Wed, 7 Oct 2026, Julius Bairaktaris wrote: > > > > > Since r16-1398, expanded blocks stay non-BB_RTL until the end of > > > expansion, but BLOCK_FOR_INSN of their insns is already set. > > > mips16_gp_pseudo_reg emits the $gp load after the basic block note of > > > such a block, and add_insn_after then crashes on BB_END. Only touch > > > BB_END and BB_HEAD of BB_RTL blocks; pass_expand::execute sets them > > > for the others from head_end_for_bb. > > > > Hmm, I think this issue might be more wide-spread and using arbitrary > > RTL APIs during RTL expansion is now dangerous. That said, > > mips16_gp_pseudo_reg should probably not do this, since try-expansion > > and throw away is expected to leave no trail. > > > > How do we get there? > > > > > Assisted-by: Claude:claude-opus-5-5 > > > > > > PR target/125582 > > > > > > gcc/ChangeLog: > > > > > > * emit-rtl.cc (add_insn_after): Only update BB_END for BB_RTL > > > blocks. > > > (add_insn_before): Only check BB_HEAD for BB_RTL blocks. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gcc.target/mips/pr125582.c: New test. > > > > > > Signed-off-by: Julius Bairaktaris <[email protected]> > > > --- > > > Bootstrapped and regtested on x86_64-pc-linux-gnu with no new failures. > > > gcc.target/mips/mips.exp on a mipsel-linux-musl cross compiler shows no > > > changes except the new test, which ICEs at -O1 and above without the > > > patch. > > > > > > gcc/emit-rtl.cc | 9 ++++++--- > > > gcc/testsuite/gcc.target/mips/pr125582.c | 25 ++++++++++++++++++++++++ > > > 2 files changed, 31 insertions(+), 3 deletions(-) > > > create mode 100644 gcc/testsuite/gcc.target/mips/pr125582.c > > > > > > diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc > > > index d064b2a1064..06294604573 100644 > > > --- a/gcc/emit-rtl.cc > > > +++ b/gcc/emit-rtl.cc > > > @@ -4373,8 +4373,10 @@ add_insn_after (rtx_insn *insn, rtx_insn *after, > > > basic_block bb) > > > if (INSN_P (insn)) > > > df_insn_rescan (insn); > > > /* Should not happen as first in the BB is always > > > - either NOTE or LABEL. */ > > > - if (BB_END (bb) == after > > > + either NOTE or LABEL. Blocks expanded but not yet BB_RTL > > > + have no BB_END. */ > > > + if ((bb->flags & BB_RTL) > > > + && BB_END (bb) == after > > > /* Avoid clobbering of structure when creating new BB. */ > > > && !BARRIER_P (insn) > > > && !NOTE_INSN_BASIC_BLOCK_P (insn)) > > > @@ -4408,7 +4410,8 @@ add_insn_before (rtx_insn *insn, rtx_insn *before, > > > basic_block bb) > > > df_insn_rescan (insn); > > > /* Should not happen as first in the BB is always either NOTE or > > > LABEL. */ > > > - gcc_assert (BB_HEAD (bb) != insn > > > + gcc_assert (!(bb->flags & BB_RTL) > > > + || BB_HEAD (bb) != insn > > > /* Avoid clobbering of structure when creating new BB. */ > > > || BARRIER_P (insn) > > > || NOTE_INSN_BASIC_BLOCK_P (insn)); > > > diff --git a/gcc/testsuite/gcc.target/mips/pr125582.c > > > b/gcc/testsuite/gcc.target/mips/pr125582.c > > > new file mode 100644 > > > index 00000000000..524ef4bdf4d > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/mips/pr125582.c > > > @@ -0,0 +1,25 @@ > > > +/* PR target/125582 */ > > > +/* { dg-do compile } */ > > > +/* { dg-options "-mabicalls -fpic -mips16" } */ > > > + > > > +int foo (void *, unsigned, unsigned); > > > + > > > +static int > > > +bar (int *p) > > > +{ > > > + int ret; > > > + > > > + do > > > + ret = foo (p, sizeof (*p), 1); > > > + while (ret == -1); > > > + > > > + return ret; > > > +} > > > + > > > +int > > > +baz (void) > > > +{ > > > + int x = 1; > > > + > > > + return bar (&x); > > > +} > > > > > > > -- > > Richard Biener <[email protected]> > > SUSE Software Solutions Germany GmbH, > > Frankenstrasse 146, 90461 Nuernberg, Germany; > > GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri; (HRB 36809, AG Nuernberg) > -- Richard Biener <[email protected]> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri; (HRB 36809, AG Nuernberg)
