Re: [PATCH] shrink-wrapping: Fix up prologue block discovery [PR103860]

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Thu, Dec 30, 2021 at 10:43:32AM +0100, Jakub Jelinek wrote:
> The following testcase is miscompiled, because a prologue which
> contains subq $8, %rsp instruction is emitted at the start of
> a basic block which contains conditional jump that depends on
> flags register set in an earlier basic block, the prologue instruction
> then clobbers those flags.
> Normally this case is checked by can_get_prologue predicate, but this
> is done only at the start of the loop.  If we update pro later in the
> loop (because some bb shouldn't be duplicated) and then don't push
> anything further into vec and the vec is already empty (this can happen
> when the new pro is already in bb_with bitmask and either has no successors
> (that is the case in the testcase where that bb ends with a trap) or
> all the successors are already in bb_with, then the loop doesn't iterate
> further and can_get_prologue will not be checked.
> 
> The following simple patch makes sure we call can_get_prologue even after
> the last former iteration when vec is already empty and only break from
> the loop afterwards (and only if the updating of pro done because of
> !can_get_prologue didn't push anything into vec again).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

That looks good, and very simple, thanks!

git blame says I wrote 69.9% of shrink-wrap.c, but I am not maintainer
of it, so I cannot approve your patch -- but it is fine afaics.


Segher


> 2021-12-30  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR rtl-optimization/103860
> 	* shrink-wrap.c (try_shrink_wrapping): Make sure can_get_prologue is
> 	called on pro even if nothing further is pushed into vec.
> 
> 	* gcc.dg/pr103860.c: New test.