[PATCH] D48721: Patch to fix pragma metadata for do-while loops

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D48721#1150677, @bjope wrote:

> In https://reviews.llvm.org/D48721#1150361, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D48721#1150333, @bjope wrote:
> >
> > > Is the fault that the metadata only should be put on the back edge, not the branch in the preheader?
> >
> >
> > Yea. Our past thinking has been that any backedge in the loop is valid. The metadata shouldn't end up other places, although it's benign unless those other places are (or may later become) a backedge for some different loop.
>
>
> I'm no expert on clang. The patch seems to fix the problem if the goal is to only add the loop-metadata on the backedge , but I'll leave it to someone else to approve it.
>
> I'm a little bit concerned about opt not detecting this kind of problems though.
>  Would it be possible for some verifier to detect if we have loop metadata on some branch that aren't in the latch block?
>  And/or should the optimization that "merges" two branches with different loop metadata), be smarter about which loop metadata to keep? Or maybe we should be defensive and discard loop metadata on the merged branch instruction?


We could add this to the verifier. We have transformation passes that aren't explicitly loop aware, and so the question is would those passes now need to do something special to remain valid in the presence of this metadata. As a general rule, metadata in invalid locations is simply ignored. It clearly is a problem, if metadata is moving from one valid location to an unrelated valid location.


Repository:
  rC Clang

https://reviews.llvm.org/D48721