Added Remarks for Loop Versioning LICM Pass

[Deepak_Porwal]

Summary

Added Remark messages for Loop Versioning LICM Pass to get the remark messages while using opt-viewer tool.

[anemet]

Thanks for working on this!

Please include diff context for the change. See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface how to generate the diff.

lib/Transforms/Scalar/LoopVersioningLICM.cpp

410

It's usually useful to output the actual numbers. See how we do that in Inliner.cpp where we include the cost and the threshold.

Also please use the new closure API with emit. Again, see the inliner or https://reviews.llvm.org/D38285.

[Deepak_Porwal]

lib/Transforms/Scalar/LoopVersioningLICM.cpp

410

Working on necessary changes.

Out of curiosity would like to know the reason for using new Closure API with emit for some places and not for all places (as in Inliner.cpp).

[Deepak_Porwal]

Incorporated changes suggested by Adam

[anemet]

lib/Transforms/Scalar/LoopVersioningLICM.cpp

410

Out of curiosity would like to know the reason for using new Closure API with emit for some places and not for all places (as in Inliner.cpp).

I only converted a few in order to test the approach. @vivekvpandya is converting most other places.

We should convert all unless the emit call is already guarded by allowExtraAnalysis. There is also one more subtle case that cannot be converted when instead of the passname PrintAll is passed.

421

Number of runtime checks (<number>) exceeds threshold (<threshold>)

498–501

Can this actually return the instruction as well and then we can use that for the location rather than the entire loop.

504–512

Same here if possible.

519

Versioned loop for LICM. Please remove the new line everywhere.

[ashutosh.nema]

Thanks for working on this !

lib/Transforms/Scalar/LoopVersioningLICM.cpp

404–405

For individual instruction you can dump the remark at this point.

[Deepak_Porwal]

As suggested by Ashutosh added remark to check legality of instruction in function 'legalLoopInstructions' .

[Deepak_Porwal]

lib/Transforms/Scalar/LoopVersioningLICM.cpp

504–512

Not possible here

[eastig]

lib/Transforms/Scalar/LoopVersioningLICM.cpp

213–215

What about to make ORE a class member?

400–402

Maybe it's better to use 'Unsafe' instead of 'Illegal'?

441–456

The message reported here is quite tricky. What I understand is: per cent of invariant memory operations in less then threshold per cent.
NV does not support FP values. Maybe it's better to add it instead of truncating 'static_cast<int>(InvariantThreshold)'. Diagnostics in this way do not provide actual information.

501–505

Do we need this remark here because remarks related to legality of instructions have already been emitted?

507–511

Maybe legalLoopMemoryAccesses should emit remarks?

[Deepak_Porwal]

Making ORE a class member and using it to emit remarks for various functions.

[Deepak_Porwal]

lib/Transforms/Scalar/LoopVersioningLICM.cpp

441–456

I am not sure how to support FP value in NV

501–505

We can keep this as it is informing about current loop location for the instruction.

507–511

It's difficult to emit remark from legalLoopMemoryAccesses as we don't have current loop info.

[ashutosh.nema]

This patch looks OK to me, except the floating point value support in NV (maybe it can be done as a later enhancement once NV supports floating point values)

If @anemet & @eastig is OK you can go ahead.

Thanks,
Ashutosh

[anemet]

lib/Transforms/Scalar/LoopVersioningLICM.cpp

441–456

That should be trivial. Take a look at how integers are emitted.

501–505

We shouldn't emit multiple remarks for a single problem, if that is what you mean. If legalLoopInstructions has already emitted something, please don't emit anything here.

If you want to mention the loop in the remark in legalLoopInstructions, you should be able to do that. We can already insert DebugLoc into the stream, there should be examples in other passes.

Alternatively, you can use the loop location for the main DebugLoc of the remark and insert the Instruction via an NV. Opt-viewer would display that as a link to the instruction in the code.

519

Can we mention how many checks we had to insert?

[eastig]

lib/Transforms/Scalar/LoopVersioningLICM.cpp

507–511

It's difficult to emit remark from legalLoopMemoryAccesses as we don't have current loop info.

Sorry, don't follow. 'legalLoopMemoryAccesses' is a member function. It has the same access to CurLoop as isLegalForVersioning.

[Deepak_Porwal]

Support for float value in NV

[Deepak_Porwal]

Ping

[eastig]

The patch looks OK to me if using std::to_string is allowed now.

lib/IR/DiagnosticInfo.cpp

172

There were some build issues with std::to_string especially with old gcc versions. I don't know if all of them have been fixed. There is llvm::to_string to work around the issues.

[Deepak_Porwal]

Changed "std::to_string" To "llvm::to_string" as suggested by Evgeny

[eastig]

LGTM

include/llvm/IR/DiagnosticInfo.h

struct Argument {
     // If set, the debug location corresponding to the value.
     DiagnosticLocation Loc;
 
     explicit Argument(StringRef Str = "") : Key("String"), Val(Str) {}
     Argument(StringRef Key, const Value *V);
     Argument(StringRef Key, const Type *T);
     Argument(StringRef Key, StringRef S);
     Argument(StringRef Key, int N);
+    Argument(StringRef Key, float N);
     Argument(StringRef Key, long N);
     Argument(StringRef Key, long long N);
     Argument(StringRef Key, unsigned N);
     Argument(StringRef Key, unsigned long N);
     Argument(StringRef Key, unsigned long long N);
     Argument(StringRef Key, bool B) : Key(Key), Val(B ? "true" : "false") {}
     Argument(StringRef Key, DebugLoc dl);
   };

lib/IR/DiagnosticInfo.cpp

 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include <atomic>
 #include <cassert>
 #include <memory>
 #include <string>
 
 using namespace llvm;
 
 int llvm::getNextAvailablePluginDiagnosticKind() {
DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, const Type *T)
   OS << *T;
 }
 
 DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, StringRef S)
     : Key(Key), Val(S.str()) {}
 
 DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, int N)
     : Key(Key), Val(itostr(N)) {}
   
+DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, float N)
+    : Key(Key), Val(llvm::to_string(N)) {} 
+
 DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, long N)
     : Key(Key), Val(itostr(N)) {}
 
 DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, long long N)
     : Key(Key), Val(itostr(N)) {}
 
 DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, unsigned N)
     : Key(Key), Val(utostr(N)) {}

lib/Transforms/Scalar/LoopVersioningLICM.cpp

 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/AliasSetTracker.h"
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/Analysis/LoopAccessAnalysis.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/LoopPass.h"
+#include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/Analysis/ScalarEvolutionExpander.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/MDBuilder.h"
void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<DominatorTreeWrapperPass>();
     AU.addRequiredID(LCSSAID);
     AU.addRequired<LoopAccessLegacyAnalysis>();
     AU.addRequired<LoopInfoWrapperPass>();
     AU.addRequiredID(LoopSimplifyID);
     AU.addRequired<ScalarEvolutionWrapperPass>();
     AU.addPreserved<AAResultsWrapperPass>();
     AU.addPreserved<GlobalsAAWrapperPass>();
+    AU.addRequired<OptimizationRemarkEmitterWrapperPass>();
   }
 
   LoopVersioningLICM()
       : LoopPass(ID), AA(nullptr), SE(nullptr), LAA(nullptr), LAI(nullptr),
         CurLoop(nullptr), LoopDepthThreshold(LVLoopDepthThreshold),
         InvariantThreshold(LVInvarThreshold), LoadAndStoreCounter(0),
-        InvariantCounter(0), IsReadOnlyLoop(true) {
+        InvariantCounter(0), IsReadOnlyLoop(true), ORE(nullptr) {
     initializeLoopVersioningLICMPass(*PassRegistry::getPassRegistry());
   }
   StringRef getPassName() const override { return "Loop Versioning for LICM"; }
 
   void reset() {
     AA = nullptr;
     SE = nullptr;
     LAA = nullptr;
     CurLoop = nullptr;
     LoadAndStoreCounter = 0;
     InvariantCounter = 0;
     IsReadOnlyLoop = true;
+    ORE = nullptr;
     CurAST.reset();
   }
 
   class AutoResetter {
   public:
     AutoResetter(LoopVersioningLICM &LVLICM) : LVLICM(LVLICM) {}
     ~AutoResetter() { LVLICM.reset(); }
 
private:
   std::unique_ptr<AliasSetTracker>
       CurAST; // AliasSet information for the current loop.
 
   unsigned LoopDepthThreshold;  // Maximum loop nest threshold
   float InvariantThreshold;     // Minimum invariant threshold
   unsigned LoadAndStoreCounter; // Counter to track num of load & store
   unsigned InvariantCounter;    // Counter to track num of invariant
   bool IsReadOnlyLoop;          // Read only loop marker.
+  OptimizationRemarkEmitter *ORE;
 
   bool isLegalForVersioning();
   bool legalLoopStructure();
   bool legalLoopInstructions();
   bool legalLoopMemoryAccesses();
   bool isLoopAlreadyVisited();
   void setNoAliasToLoop(Loop *);
   bool instructionSafeForVersioning(Instruction *);
 };
 }
 
 /// \brief Check loop structure and confirms it's good for LoopVersioningLICM.

 
 /// \brief Check loop instructions and confirms it's good for
 /// LoopVersioningLICM.
 bool LoopVersioningLICM::legalLoopInstructions() {
   // Resetting counters.
   LoadAndStoreCounter = 0;
   InvariantCounter = 0;
   IsReadOnlyLoop = true;
+  using namespace ore;
+
   // Iterate over loop blocks and instructions of each block and check
   // instruction safety.
   for (auto *Block : CurLoop->getBlocks())
     for (auto &Inst : *Block) {
       // If instruction is unsafe just return false.
-      if (!instructionSafeForVersioning(&Inst))
+      if (!instructionSafeForVersioning(&Inst)) {
+        ORE->emit([&]() {
+          return OptimizationRemarkMissed(DEBUG_TYPE, "IllegalLoopInst",
+                                 &Inst)
+          << " Unsafe Loop Instruction";
+        });
         return false;
       }
+    }
   // Get LoopAccessInfo from current loop.
   LAI = &LAA->getInfo(CurLoop);
   // Check LoopAccessInfo for need of runtime check.
   if (LAI->getRuntimePointerChecking()->getChecks().empty()) {
     DEBUG(dbgs() << "    LAA: Runtime check not found !!\n");
     return false;
   }
   // Number of runtime-checks should be less then RuntimeMemoryCheckThreshold
   if (LAI->getNumRuntimePointerChecks() >
       VectorizerParams::RuntimeMemoryCheckThreshold) {
     DEBUG(dbgs() << "    LAA: Runtime checks are more than threshold !!\n");
+    ORE->emit([&]() {
+        return OptimizationRemarkMissed(DEBUG_TYPE, "RuntimeCheck",
+                                 CurLoop->getStartLoc(), CurLoop->getHeader())
+        << "Number of runtime checks " << NV("RuntimeChecks",
+                                             LAI->getNumRuntimePointerChecks())
+        << " exceeds threshold "
+        << NV("Threshold", VectorizerParams::RuntimeMemoryCheckThreshold);
+    });
     return false;
   }
   // Loop should have at least one invariant load or store instruction.
   if (!InvariantCounter) {
     DEBUG(dbgs() << "    Invariant not found !!\n");
     return false;
   }
   // Read only loop not allowed.
   if (IsReadOnlyLoop) {
     DEBUG(dbgs() << "    Found a read-only loop!\n");
     return false;
   }
   // Profitablity check:
   // Check invariant threshold, should be in limit.
   if (InvariantCounter * 100 < InvariantThreshold * LoadAndStoreCounter) {
     DEBUG(dbgs()
           << "    Invariant load & store are less then defined threshold\n");
     DEBUG(dbgs() << "    Invariant loads & stores: "
                  << ((InvariantCounter * 100) / LoadAndStoreCounter) << "%\n");
     DEBUG(dbgs() << "    Invariant loads & store threshold: "
                  << InvariantThreshold << "%\n");
+    ORE->emit([&]() {
+        return OptimizationRemarkMissed(DEBUG_TYPE, "InvariantThreshold",
+                                 CurLoop->getStartLoc(), CurLoop->getHeader())
+        << "Invariant load & store "
+        << NV("LoadAndStoreCounter", 
+              ((InvariantCounter * 100)/LoadAndStoreCounter))
+        << " are less then defined threshold "
+        << NV("Threshold", InvariantThreshold);
+    });
     return false;
   }
   return true;
 }
 
 /// \brief It checks loop is already visited or not.
 /// check loop meta data, if loop revisited return true
 /// else false.
 bool LoopVersioningLICM::isLoopAlreadyVisited() {
   // Check LoopVersioningLICM metadata into loop
   if (findStringMetadataForLoop(CurLoop, LICMVersioningMetaData)) {
     return true;
   }
   return false;
 }
 
 /// \brief Checks legality for LoopVersioningLICM by considering following:
 /// a) loop structure legality   b) loop instruction legality
 /// c) loop memory access legality.
 /// Return true if legal else returns false.
 bool LoopVersioningLICM::isLegalForVersioning() {
+  using namespace ore;
+
   DEBUG(dbgs() << "Loop: " << *CurLoop);
   // Make sure not re-visiting same loop again.
   if (isLoopAlreadyVisited()) {
     DEBUG(
         dbgs() << "    Revisiting loop in LoopVersioningLICM not allowed.\n\n");
     return false;
   }
   // Check loop structure leagality.
   if (!legalLoopStructure()) {
     DEBUG(
         dbgs() << "    Loop structure not suitable for LoopVersioningLICM\n\n");
+    ORE->emit([&]() {
+        return OptimizationRemarkMissed(DEBUG_TYPE, "IllegalLoopStruct",
+                                 CurLoop->getStartLoc(), CurLoop->getHeader())
+        << " Unsafe Loop structure";
+    });
     return false;
   }
   // Check loop instruction leagality.
   if (!legalLoopInstructions()) {
-    DEBUG(dbgs()
+    DEBUG(dbgs() 
           << "    Loop instructions not suitable for LoopVersioningLICM\n\n");
     return false;
-  }
+  } 
   // Check loop memory access leagality.
   if (!legalLoopMemoryAccesses()) {
     DEBUG(dbgs()
           << "    Loop memory access not suitable for LoopVersioningLICM\n\n");
+    ORE->emit([&]() {
+        return OptimizationRemarkMissed(DEBUG_TYPE, "IllegalLoopMemoryAccess",
+                                 CurLoop->getStartLoc(), CurLoop->getHeader())
+        << " Unsafe Loop memory access";
+    });
     return false;
   }
   // Loop versioning is feasible, return true.
   DEBUG(dbgs() << "    Loop Versioning found to be beneficial\n\n");
+  ORE->emit([&]() {
+      return OptimizationRemark(DEBUG_TYPE, "IsLegalForVersioning",
+                               CurLoop->getStartLoc(), CurLoop->getHeader())
+      << " Versioned loop for LICM."
+      << " Number of runtime checks we had to insert " << NV("RuntimeChecks",
+                                     LAI->getNumRuntimePointerChecks());
+  });
   return true;
 }
 
 /// \brief Update loop with aggressive aliasing assumptions.
 /// It marks no-alias to any pairs of memory operations by assuming
 /// loop should not have any must-alias memory accesses pairs.
 /// During LoopVersioningLICM legality we ignore loops having must
 /// aliasing memory accesses.
bool LoopVersioningLICM::runOnLoop(Loop *L, LPPassManager &LPM) {
 
   if (skipLoop(L))
     return false;
   // Get Analysis information.
   AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
   SE = &getAnalysis<ScalarEvolutionWrapperPass>().getSE();
   LAA = &getAnalysis<LoopAccessLegacyAnalysis>();
   LAI = nullptr;
+  ORE = &getAnalysis<OptimizationRemarkEmitterWrapperPass>().getORE();
   // Set Current Loop
   CurLoop = L;
   CurAST.reset(new AliasSetTracker(*AA));
 
   // Loop over the body of this loop, construct AST.
   LoopInfo *LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
   for (auto *Block : L->getBlocks()) {
     if (LI->getLoopFor(Block) == L) // Ignore blocks in subloop.

 INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(GlobalsAAWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(LCSSAWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(LoopAccessLegacyAnalysis)
 INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(LoopSimplify)
 INITIALIZE_PASS_DEPENDENCY(ScalarEvolutionWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(OptimizationRemarkEmitterWrapperPass)
 INITIALIZE_PASS_END(LoopVersioningLICM, "loop-versioning-licm",
                     "Loop Versioning For LICM", false, false)
 
 Pass *llvm::createLoopVersioningLICMPass() { return new LoopVersioningLICM(); }