[LoopUnswitch] Use reference variables instead of pointer one

[jaykang10]

Summary

Following comment of @fhahn on https://reviews.llvm.org/D99490#inline-935642, used reference variables instead of pointer one

[fhahn]

llvm/lib/Transforms/Scalar/LoopUnswitch.cpp

851

nit: no llvm:: prefix should be needed. (e.g. we are also not using llvm::Instruction)

llvm/lib/Transforms/Utils/LoopUtils.cpp

1708–1709

Not sure if the comment should be there. It should be in the header (which is what doxygen uses) and duplicating it here means it will probably diverge in the future, which is even more confusing.

[jaykang10]

llvm/lib/Transforms/Scalar/LoopUnswitch.cpp

851

You are right!!! I will update it.

llvm/lib/Transforms/Utils/LoopUtils.cpp

1708–1709

Yep, I will remove it.

[jaykang10]

Following comment of @fhahn, updated code.

[jaykang10]

@fhahn If you need something more for this one, please let me know.

[fhahn]

LGTM, thanks

llvm/include/llvm/Transforms/Utils/LoopUtils.h

 /// modify the memory feeding the condition, perform 'partial unswitching'. That
 /// is, duplicate the instructions feeding the condition in the pre-header. Then
 /// unswitch on the duplicated condition. The condition is now known in the
 /// unswitched version for the 'invariant' path through the original loop.
 ///
 /// If the branch condition of the header is partially invariant, return a pair
 /// containing the instructions to duplicate and a boolean Constant to update
 /// the condition in the loops created for the true or false successors.
-Optional<IVConditionInfo> hasPartialIVCondition(Loop *L, unsigned MSSAThreshold,
-                                                MemorySSA *MSSA, AAResults *AA);
+Optional<IVConditionInfo> hasPartialIVCondition(Loop &L, unsigned MSSAThreshold,
+                                                MemorySSA &MSSA, AAResults &AA);
 
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_UTILS_LOOPUTILS_H

llvm/lib/Transforms/Scalar/LoopUnswitch.cpp

bool LoopUnswitch::processCurrentLoop() {
   // Check if there is a header condition that is invariant along the patch from
   // either the true or false successors to the header. This allows unswitching
   // conditions depending on memory accesses, if there's a path not clobbering
   // the memory locations. Check if this transform has been disabled using
   // metadata, to avoid unswitching the same loop multiple times.
   if (MSSA &&
       !findOptionMDForLoop(CurrentLoop, "llvm.loop.unswitch.partial.disable")) {
     if (auto Info =
-            llvm::hasPartialIVCondition(CurrentLoop, MSSAThreshold, MSSA, AA)) {
+            hasPartialIVCondition(*CurrentLoop, MSSAThreshold, *MSSA, *AA)) {
       assert(!Info->InstToDuplicate.empty() &&
              "need at least a partially invariant condition");
       LLVM_DEBUG(dbgs() << "loop-unswitch: Found partially invariant condition "
                         << *Info->InstToDuplicate[0] << "\n");
 
       Instruction *TI = CurrentLoop->getHeader()->getTerminator();
       Value *LoopCond = Info->InstToDuplicate[0];
 

llvm/lib/Transforms/Utils/LoopUtils.cpp

std::pair<Instruction *, Instruction *> llvm::addRuntimeChecks(
   // constant expression in which case there is no Instruction anchored in a
   // the block.
   Instruction *Check =
       BinaryOperator::CreateAnd(MemoryRuntimeCheck, ConstantInt::getTrue(Ctx));
   ChkBuilder.Insert(Check, "memcheck.conflict");
   FirstInst = GetFirstInst(FirstInst, Check, Loc);
   return std::make_pair(FirstInst, Check);
 }
 
-/// Check if the loop header has a conditional branch that is not
-/// loop-invariant, because it involves load instructions. If all paths from
-/// either the true or false successor to the header or loop exists do not
-/// modify the memory feeding the condition, perform 'partial unswitching'. That
-/// is, duplicate the instructions feeding the condition in the pre-header. Then
-/// unswitch on the duplicated condition. The condition is now known in the
-/// unswitched version for the 'invariant' path through the original loop.
-///
-/// If the branch condition of the header is partially invariant, return a pair
-/// containing the instructions to duplicate and a boolean Constant to update
-/// the condition in the loops created for the true or false successors.
-Optional<IVConditionInfo> llvm::hasPartialIVCondition(Loop *L,
+Optional<IVConditionInfo> llvm::hasPartialIVCondition(Loop &L,
                                                       unsigned MSSAThreshold,
-                                                      MemorySSA *MSSA,
-                                                      AAResults *AA) {
-  auto *TI = dyn_cast<BranchInst>(L->getHeader()->getTerminator());
+                                                      MemorySSA &MSSA,
+                                                      AAResults &AA) {
+  auto *TI = dyn_cast<BranchInst>(L.getHeader()->getTerminator());
   if (!TI || !TI->isConditional())
     return {};
 
   auto *CondI = dyn_cast<CmpInst>(TI->getCondition());
   // The case with the condition outside the loop should already be handled
   // earlier.
-  if (!CondI || !L->contains(CondI))
+  if (!CondI || !L.contains(CondI))
     return {};
 
   SmallVector<Instruction *> InstToDuplicate;
   InstToDuplicate.push_back(CondI);
 
   SmallVector<Value *, 4> WorkList;
   WorkList.append(CondI->op_begin(), CondI->op_end());
 
   SmallVector<MemoryAccess *, 4> AccessesToCheck;
   SmallVector<MemoryLocation, 4> AccessedLocs;
   while (!WorkList.empty()) {
     Instruction *I = dyn_cast<Instruction>(WorkList.pop_back_val());
-    if (!I || !L->contains(I))
+    if (!I || !L.contains(I))
       continue;
 
     // TODO: support additional instructions.
     if (!isa<LoadInst>(I) && !isa<GetElementPtrInst>(I))
       return {};
 
     // Do not duplicate volatile and atomic loads.
     if (auto *LI = dyn_cast<LoadInst>(I))
       if (LI->isVolatile() || LI->isAtomic())
         return {};
 
     InstToDuplicate.push_back(I);
-    if (MemoryAccess *MA = MSSA->getMemoryAccess(I)) {
+    if (MemoryAccess *MA = MSSA.getMemoryAccess(I)) {
       if (auto *MemUse = dyn_cast_or_null<MemoryUse>(MA)) {
         // Queue the defining access to check for alias checks.
         AccessesToCheck.push_back(MemUse->getDefiningAccess());
         AccessedLocs.push_back(MemoryLocation::get(I));
       } else {
         // MemoryDefs may clobber the location or may be atomic memory
         // operations. Bail out.
         return {};
       }
     }
     WorkList.append(I->op_begin(), I->op_end());
   }
 
   if (InstToDuplicate.empty())
     return {};
 
   SmallVector<BasicBlock *, 4> ExitingBlocks;
-  L->getExitingBlocks(ExitingBlocks);
+  L.getExitingBlocks(ExitingBlocks);
   auto HasNoClobbersOnPath =
-      [L, AA, &AccessedLocs, &ExitingBlocks, &InstToDuplicate,
+      [&L, &AA, &AccessedLocs, &ExitingBlocks, &InstToDuplicate,
        MSSAThreshold](BasicBlock *Succ, BasicBlock *Header,
                       SmallVector<MemoryAccess *, 4> AccessesToCheck)
       -> Optional<IVConditionInfo> {
     IVConditionInfo Info;
     // First, collect all blocks in the loop that are on a patch from Succ
     // to the header.
     SmallVector<BasicBlock *, 4> WorkList;
     WorkList.push_back(Succ);
     WorkList.push_back(Header);
     SmallPtrSet<BasicBlock *, 4> Seen;
     Seen.insert(Header);
     Info.PathIsNoop &=
         all_of(*Header, [](Instruction &I) { return !I.mayHaveSideEffects(); });
 
     while (!WorkList.empty()) {
       BasicBlock *Current = WorkList.pop_back_val();
-      if (!L->contains(Current))
+      if (!L.contains(Current))
         continue;
       const auto &SeenIns = Seen.insert(Current);
       if (!SeenIns.second)
         continue;
 
       Info.PathIsNoop &= all_of(
           *Current, [](Instruction &I) { return !I.mayHaveSideEffects(); });
       WorkList.append(succ_begin(Current), succ_end(Current));
while (!AccessesToCheck.empty()) {
 
       // MemoryUse are read-only accesses.
       if (isa<MemoryUse>(Current))
         continue;
 
       // For a MemoryDef, check if is aliases any of the location feeding
       // the original condition.
       if (auto *CurrentDef = dyn_cast<MemoryDef>(Current)) {
-        if (any_of(AccessedLocs, [AA, CurrentDef](MemoryLocation &Loc) {
+        if (any_of(AccessedLocs, [&AA, CurrentDef](MemoryLocation &Loc) {
               return isModSet(
-                  AA->getModRefInfo(CurrentDef->getMemoryInst(), Loc));
+                  AA.getModRefInfo(CurrentDef->getMemoryInst(), Loc));
             }))
           return {};
       }
 
       for (Use &U : Current->uses())
         AccessesToCheck.push_back(cast<MemoryAccess>(U.getUser()));
     }
 
     // We could also allow loops with known trip counts without mustprogress,
     // but ScalarEvolution may not be available.
     Info.PathIsNoop &=
-        L->getHeader()->getParent()->mustProgress() || hasMustProgress(L);
+        L.getHeader()->getParent()->mustProgress() || hasMustProgress(&L);
 
     // If the path is considered a no-op so far, check if it reaches a
     // single exit block without any phis. This ensures no values from the
     // loop are used outside of the loop.
     if (Info.PathIsNoop) {
       for (auto *Exiting : ExitingBlocks) {
         if (!Seen.contains(Exiting))
           continue;
         for (auto *Succ : successors(Exiting)) {
-          if (L->contains(Succ))
+          if (L.contains(Succ))
             continue;
 
           Info.PathIsNoop &= llvm::empty(Succ->phis()) &&
                              (!Info.ExitForPath || Info.ExitForPath == Succ);
           if (!Info.PathIsNoop)
             break;
           assert((!Info.ExitForPath || Info.ExitForPath == Succ) &&
                  "cannot have multiple exit blocks");
           Info.ExitForPath = Succ;
         }
       }
     }
     if (!Info.ExitForPath)
       Info.PathIsNoop = false;
 
     Info.InstToDuplicate = InstToDuplicate;
     return Info;
   };
 
   // If we branch to the same successor, partial unswitching will not be
   // beneficial.
   if (TI->getSuccessor(0) == TI->getSuccessor(1))
     return {};
 
-  if (auto Info = HasNoClobbersOnPath(TI->getSuccessor(0), L->getHeader(),
+  if (auto Info = HasNoClobbersOnPath(TI->getSuccessor(0), L.getHeader(),
                                       AccessesToCheck)) {
     Info->KnownValue = ConstantInt::getTrue(TI->getContext());
     return Info;
   }
-  if (auto Info = HasNoClobbersOnPath(TI->getSuccessor(1), L->getHeader(),
+  if (auto Info = HasNoClobbersOnPath(TI->getSuccessor(1), L.getHeader(),
                                       AccessesToCheck)) {
     Info->KnownValue = ConstantInt::getFalse(TI->getContext());
     return Info;
   }
 
   return {};
 }