Segher Boessenkool <seg...@kernel.crashing.org> writes:
> On Thu, Feb 23, 2017 at 04:27:26PM +0000, Toma Tabacu wrote:
> > > This happens when you have inserted code ending in a jump on an
> edge.
> > > This then will need updating of the CFG, and this code does not know
> > > how to do that.
> >
> > Would the following be an appropriate solution ?
>
> [ snip ]
>
> > @@ -2047,6 +2047,16 @@ commit_one_edge_insertion (edge e)
> > + /* If the edge contains control flow instructions, remember to
> update the
> > + CFG after the insertion. */
>
> I don't know what all can break if you allow control flow insns in
> insert_insn_on_edge -- this function is called from many different
> passes, many things can break -- but this does look like it should work.
>
> > + bool update_cfg = false;
> > + for (tmp = insns; tmp && update_cfg == false; tmp = NEXT_INSN
> (tmp))
> > + if (control_flow_insn_p (tmp))
> > + {
> > + update_cfg = true;
> > + break;
> > + }
>
> > + if (update_cfg)
> > + {
> > + auto_sbitmap blocks (last_basic_block_for_fn (cfun));
> > + bitmap_ones (blocks);
> > + find_many_sub_basic_blocks (blocks);
> > +
> > + last = BB_END (bb);
> > + }
>
> Maybe you can keep track of what blocks to split, instead of just saying
> "all".
>
> > In short, I'm updating the CFG by calling find_many_sub_basic_blocks
> > with an all-one block bitmap (this also happens in cfgexpand.c, after
> > the edge
> > insertions) whenever an edge contains an insn which satisfies
> control_flow_insn_p.
>
> General... Patches need to go to gcc-patches@. You also should have
> your copyright assignment in order (I have no idea if you do; if you do,
> please ignore). Finally, trunk currently is in stage 4, this work will
> need to wait for stage 1 (a couple of months, something like that).
This is an ICE that will be reproducible on a primary target so is still
appropriate to pursue in stage4 as far as I understand. I'm hoping to
find time to work with Toma on this issue.
> Can't whatever creates those jump insns keep the cfg in shape? That
> would avoid all issues here.
The problem, I think, is that these instructions are not yet in the cfg
and are being inserted on an edge. The jump is produced from the inline
memcpy expansion we do for MIPS. In some cases there will be no loop,
some cases there will be a loop ending with the conditional jump and
some cases will have a loop and other instructions after the conditional
jump. The 1st and 3rd form will get through the logic in
commit_one_edge_insertion (albeit that the 3rd form will have incorrect
cfg actually) but the 2nd form is rejected because of ending with a jump.
Other than coping with the potential for sub blocks here or letting them
through and leaving later code to split the blocks then I see no other
way forward. I agree it should be possible to process just the blocks
with jump instructions in the middle and that is actually going to be
exactly one block in this case. I don't know if updating the CFG while
edges are being iterated on in commit_edge_insertions is safe though
and am somewhat out of my comfort zone in general!
Thanks,
Matthew
