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

Bjorn Pettersson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 5 10:14:35 PDT 2018


bjope added a comment.

In https://reviews.llvm.org/D48721#1153138, @deepak2427 wrote:

> I have updated the test to not run the optimizer. The test I had added previously for checking if the unroller is respecting the pragma is useful I think. Not sure where that can be added though. 
>  I guess it's independent of this patch anyway. If the patch and test is okay, will update bugzilla as well.


My idea was to forbid the IR that caused problems for the unroller (and other optimization passes). Then we do not need to fixup various passes to handle "misplaced" llvm.loop annotations correctly. And neither do we need to implement test cases to verify that the "misplaced" annotations are handled correctly for lots of passes.

I played around a little bit with teaching the IR Verifier in opt to check that !llvm.loop only is put in loop latches.
Something like this might do the trick, when added to Verifier::visitInstruction in lib/IR/Verifier.cpp:

  #include "llvm/Analysis/LoopInfo.h"
  
  if (I.getMetadata(LLVMContext::MD_loop)) {
    // FIXME: Is SwitchInst also allowed?
    Assert(isa<BranchInst>(I), "llvm.loop only expected on branches", &I);
    LoopInfo LI;
    LI.analyze(DT);
    Loop* L = LI.getLoopFor(BB);
    Assert(L, "llvm.loop not in a loop", &I);
    Assert(L->isLoopLatch(BB), "llvm.loop not in a latch", &I);
  }

Note that the above just was a simple test. We do not want to reinitialize LoopInfo for each found occurence of !llvm.loop.
Another problem is that the Verifier is used by lots of tools that currently do not link with LoopInfo from the Analysis code module.

So maybe this should go into the LoopVerifier pass instead (although I'm not sure if that is commonly used).
Or is there some other place where we can do it? Or some other way to do a similar check (without using LoopInfo)?

I can bring this up on llvm-dev, to get some more feedback on the idea of having a verifier for this, and how to do it properly.


https://reviews.llvm.org/D48721





More information about the cfe-commits mailing list