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

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

In D101142#2711453 <https://reviews.llvm.org/D101142#2711453>, @lebedev.ri wrote:

> 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?

This is what my example was trying to show. A latch that previously had no metadata must not be assigned metadata from a different loop.

In D101142#2711513 <https://reviews.llvm.org/D101142#2711513>, @jeroen.dobbelaere wrote:

> It indeed feels weird and maybe your testing shows that it is not that big of a problem. My feeling was that maybe a different approach in describing loop information might be more appropriate, resulting in accurate information, without the observed performance impact. That given, I have no idea yet how that could look like :(.

This is not about preserving loop metadata, but ensuring that the loop does not get assigned the wrong metadata. Like a `mustprogress` annotation or a `#pragma clang loop vectorize(assume_safety)` from a different loop.

I agree that a different approach might be better, such as Johannes' suggestion to allow Metadata on (Header) BasicBlocks (which may come with other problems), or introducing an intrinsic that holds that metadata (which comes with a performance penalty of a different kind: every other pass has to iterate over ir). But until this happens, this fixes a potential miscompilation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101142