Re: [PATCH] Fix combiner ICEs after my recent patch (PR rtl-optimization/48549)

[Eric Botcazou <ebotcazou@adacore.com>]

> That is because propagate_for_debug stops at the insn before last, doesn't
> process last any longer.  So, if there is a DEBUG_INSN right before the
> jump being deleted, and it has been propagated into, after the jump is
> deleted and retry is done at i2 earlier, following propagate_for_debug
> won't see that DEBUG_INSN any longer.

Ah, yes, thanks.

> We could certainly change propagate_for_debug, so that it would stop either
> before end (NEXT_INSN (BB_END (this_basic_block)) added in this patch), or
> after processing last (which doesn't change anything in the non-deleted
> insn case, because i3 from any try_combine surely isn't a DEBUG_INSN and
> propagate_for_debug only propagates into those), then on deletion set it to
> PREV_INSN instead of NEXT_INSN.  I wonder whether we couldn't be deleting
> the first insn in a basic block, but probably there should be at least a
> note before it.

This looks fine to me.

> Perhaps instead of the assert we could in that case just set
> last_combined_insn to insn.

Yes, this looks preferable to me, IMO we don't need to stop the compiler when 
something goes wrong with debug insns.

> 2011-04-12  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR rtl-optimization/48549
> 	* combine.c (propagate_for_debug): Also stop after BB_END of
> 	this_basic_block.  Process LAST and just stop processing after it.
> 	(combine_instructions): If last_combined_insn has been deleted,
> 	set last_combined_insn to its PREV_INSN.
>
> 	* g++.dg/opt/pr48549.C: New test.

So OK modulo the last point, thanks.

-- 
Eric Botcazou