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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to