On Thu, Oct 08, 2026 at 10:05:04AM +0000, Julius Bairaktaris wrote:
> On Thu, 8 Oct 2026, Richard Biener wrote:
> > 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?
>
> mips16_gp_pseudo_reg currently uses emit_insn_after (set, scan), which
> ends in add_insn_after, and that writes BB_END of the non-BB_RTL block.
I think the function is just completely broken.
What does it try to do and why?
IMHO all targets using push_topmost_sequence are doing a wrong thing,
what they should do instead if they really mean to emit at the start
of a function is just
insert_insn_on_edge (new_insn, single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN
(cfun)));
or
prepend_insn_on_edge (new_insn, single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN
(cfun)));
cfgexpand.cc (pass_expand::execute) calls commit_edge_insertions near the
end of the pass.
Normally, bb is set on insns only at the end of expand_gimple_basic_block
where we have insns for that bb already ready, so adding stuff within the
current bb is fine.
Looking around at the push_topmost_sequence, the uses are really creative.
I see
emit_insn_after (seq, entry_of_function ());
and
emit_insn_before (seq, NEXT_INSN (entry_of_function ()));
and
emit_insn_after (pat, get_insns ());
and then this mips bogosity (plus various cases of push_topmost_sequence
which just want to read-only inspect all insns (that is fine) or delete
some of them (mips again, that is less desirable if it is on bb boundaries
during expansion).
Now, typically the start of the topmost sequence will be the bb 2 start,
something like
(note 1 0 3 NOTE_INSN_DELETED)
(note 3 1 2 5 [bb 5] NOTE_INSN_BASIC_BLOCK)
(note 2 3 4 5 NOTE_INSN_FUNCTION_BEG)
(note 4 2 0 [bb 2] NOTE_INSN_BASIC_BLOCK)
The first note is from pass_expand::execute doing
emit_note (NOTE_INSN_DELETED);
second and third is from construct_init_block () call in there and
the last note is already from expand_gimple_basic_block on the original
GIMPLE bb 2 (successor of ENTRY at that point). In testcases like you've
provided, bb 2 can be empty and bb 3 can be a head of a loop, so in the
end a CODE_LABEL is emitted there.
If a function has parameters, there are usually insns to setup arguments
in between that first NOTE_INSN_BASIC_BLOCK and NOTE_INSN_FUNCTION_BEG.
Now, entry_of_function () is the first NOTE_INSN_BASIC_BLOCK, the init_block
is BB_RTL (like ENTRY and EXIT blocks). So, I think
emit_insn_after (seq, entry_of_function ());
is reasonable.
emit_insn_before (seq, NEXT_INSN (entry_of_function ()));
less so, there could be no NEXT_INSN after it at least in theory, but
because of the NOTE_INSN_FUNCTION_BEG, that still likely works fine too.
emit_insn_after (pat, get_insns ());
looks wrong to me, it emits insns after the NOTE_INSN_DELETED, before
NOTE_INSN_BASIC_BLOCK which should start a bb.
Now, let's look at the mips uses which are totally insane:
push_topmost_sequence ();
scan = get_insns ();
while (NEXT_INSN (scan) && !INSN_P (NEXT_INSN (scan)))
scan = NEXT_INSN (scan);
...
emit_insn_after (set, scan);
So, what does this do? It skips all NOTEs/BARRIERs/CODE_LABELs from the start
of the function, until it reaches some INSN, JUMP_INSN, CALL_INSN or
DEBUG_INSN (if at all) and then emits before it. So, if you are lucky
and function has arguments (in the testcase there is during expand
just main which doesn't have them), it is emitted before them, but it can
as well end up in a completely different bb, given that it skips over
CODE_LABELs too, it can be some multiply executed bb, etc.
So, I'd strongly recommend inserting it on edge, even emit_insn_at_entry
should work given that the first bb is init_block and BB_RTL.
Or just use what other backends are doing,
emit_insn_after (set, entry_of_function ());
Jakub
