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

[Jeff Law <jeffreyalaw@gmail.com>]

On 12/30/2021 3:08 AM, Segher Boessenkool wrote:
> 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.
Well, we should probably fix the maintainer status of shrink-wrap.c.

In the mean time, based on Segher's review, this is fine for the trunk.

jeff