[PATCH] D101142: [SimplifyCFG/JumpThreading] Do not simplify empty blocks with unconditional branches if this causes loop metadata confusion.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D101142#2711394 <https://reviews.llvm.org/D101142#2711394>, @Meinersbur wrote:

> In D101142#2711283 <https://reviews.llvm.org/D101142#2711283>, @jeroen.dobbelaere wrote:
>
>> Did you verify the effect of this on the performance ? In a similar (but different) trial where I tried to block merging of blocks when that would result in a conflict with loop annotations, I noticed unacceptable regressions.
>
> I did not. This is a correctness fix. Arguing for performance over correctness feels weird.
>
> In D101142#2711289 <https://reviews.llvm.org/D101142#2711289>, @lebedev.ri wrote:
>
>> Patch's subject is wrong. This doesn't "Transfer loop metadata to new latch.", it prevents folding.
>
> Fixed title.
>
>> if you already have metadata, why do you need `LoopHeaders` ?
>
> To identify which blocks are latches. If moving metadata from another loop there may cause correctness problems, for instance if a `mustprogress` annotation is associated with a loop that does not need to progress.
>
> Unfortunately the metadata itself is not sufficient. Let A and B be to blocks which are latches of two different loops. A has metadata, but B has not. Combining the blocks and using the metadata of A for it will be wrong, as it would apply the same metadata (e.g. `mustprogress`) to both loops, as happened in the PR50060. The alternative would be to not combine any blocks if any of the two has loop metadata. Or dropping both metadata.

What i meant is, doesn't that metadata belong to the loop latches?
So if we really want to treat loop latches specially, isn't merely the presence of said metadata enough?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101142/new/

https://reviews.llvm.org/D101142