Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC][DebugInfo] Use iterators for instruction insertion in more places #124291

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

jmorse
Copy link
Member

@jmorse jmorse commented Jan 24, 2025

As part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing methods that use Instruction*'s as positions with iterators. This patch changes some more complex call-sites, those crossing file boundaries and where I've had to perform some minor rewrites.

As part of the "RemoveDIs" work to eliminate debug intrinsics, we're
replacing methods that use Instruction*'s as positions with iterators. This
patch changes some more complex call-sites, those crossing file boundaries
and where I've had to perform some minor rewrites.
@llvmbot
Copy link
Member

llvmbot commented Jan 24, 2025

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-compiler-rt-sanitizer

@llvm/pr-subscribers-backend-hexagon

Author: Jeremy Morse (jmorse)

Changes

As part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing methods that use Instruction*'s as positions with iterators. This patch changes some more complex call-sites, those crossing file boundaries and where I've had to perform some minor rewrites.


Full diff: https://github.com/llvm/llvm-project/pull/124291.diff

10 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenFunction.h (+1-1)
  • (modified) llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h (+3-3)
  • (modified) llvm/lib/CodeGen/CodeGenPrepare.cpp (+11-11)
  • (modified) llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp (+7-7)
  • (modified) llvm/lib/Transforms/Coroutines/CoroSplit.cpp (+9-7)
  • (modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+1-1)
  • (modified) llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp (+1-1)
  • (modified) llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp (+1-1)
  • (modified) llvm/lib/Transforms/Utils/BasicBlockUtils.cpp (+6-6)
  • (modified) llvm/lib/Transforms/Utils/InlineFunction.cpp (+4-4)
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index fab27d4c22ed80..bdd6e3bd55a7fd 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -451,7 +451,7 @@ class CodeGenFunction : public CodeGenTypeCache {
              "EBB should be entry block of the current code gen function");
       PostAllocaInsertPt = AllocaInsertPt->clone();
       PostAllocaInsertPt->setName("postallocapt");
-      PostAllocaInsertPt->insertAfter(AllocaInsertPt);
+      PostAllocaInsertPt->insertAfter(AllocaInsertPt->getIterator());
     }
 
     return PostAllocaInsertPt;
diff --git a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
index b447942ffbd676..4c717af0e501f3 100644
--- a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
@@ -540,7 +540,7 @@ inline void SplitBlockAndInsertIfThenElse(Value *Cond, Instruction *SplitBefore,
 /// SplitBefore.  Returns the first insert point in the loop body, and the
 /// PHINode for the induction variable (i.e. "i" above).
 std::pair<Instruction*, Value*>
-SplitBlockAndInsertSimpleForLoop(Value *End, Instruction *SplitBefore);
+SplitBlockAndInsertSimpleForLoop(Value *End, BasicBlock::iterator SplitBefore);
 
 /// Utility function for performing a given action on each lane of a vector
 /// with \p EC elements.  To simplify porting legacy code, this defaults to
@@ -551,7 +551,7 @@ SplitBlockAndInsertSimpleForLoop(Value *End, Instruction *SplitBefore);
 /// given index, and a value which is (at runtime) the index to access.
 /// This index *may* be a constant.
 void SplitBlockAndInsertForEachLane(ElementCount EC, Type *IndexTy,
-    Instruction *InsertBefore,
+    BasicBlock::iterator InsertBefore,
     std::function<void(IRBuilderBase&, Value*)> Func);
 
 /// Utility function for performing a given action on each lane of a vector
@@ -563,7 +563,7 @@ void SplitBlockAndInsertForEachLane(ElementCount EC, Type *IndexTy,
 /// the given index, and a value which is (at runtime) the index to access. This
 /// index *may* be a constant.
 void SplitBlockAndInsertForEachLane(
-    Value *End, Instruction *InsertBefore,
+    Value *End, BasicBlock::iterator InsertBefore,
     std::function<void(IRBuilderBase &, Value *)> Func);
 
 /// Check whether BB is the merge point of a if-region.
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 7e9d705a7bef6c..94101ccca1096a 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -2935,13 +2935,13 @@ bool CodeGenPrepare::dupRetToEnableTailCallOpts(BasicBlock *BB,
 
   // Make sure there are no instructions between the first instruction
   // and return.
-  const Instruction *BI = BB->getFirstNonPHI();
+  BasicBlock::const_iterator BI = BB->getFirstNonPHIIt();
   // Skip over debug and the bitcast.
-  while (isa<DbgInfoIntrinsic>(BI) || BI == BCI || BI == EVI ||
-         isa<PseudoProbeInst>(BI) || isLifetimeEndOrBitCastFor(BI) ||
-         isFakeUse(BI))
-    BI = BI->getNextNode();
-  if (BI != RetI)
+  while (isa<DbgInfoIntrinsic>(BI) || &*BI == BCI || &*BI == EVI ||
+         isa<PseudoProbeInst>(BI) || isLifetimeEndOrBitCastFor(&*BI) ||
+         isFakeUse(&*BI))
+    BI = std::next(BI);
+  if (&*BI != RetI)
     return false;
 
   /// Only dup the ReturnInst if the CallInst is likely to be emitted as a tail
@@ -3265,8 +3265,8 @@ class TypePromotionTransaction {
     /// Either an instruction:
     /// - Is the first in a basic block: BB is used.
     /// - Has a previous instruction: PrevInst is used.
-    union {
-      Instruction *PrevInst;
+    struct {
+      BasicBlock::iterator PrevInst;
       BasicBlock *BB;
     } Point;
     std::optional<DbgRecord::self_iterator> BeforeDbgRecord = std::nullopt;
@@ -3286,7 +3286,7 @@ class TypePromotionTransaction {
         BeforeDbgRecord = Inst->getDbgReinsertionPosition();
 
       if (HasPrevInstruction) {
-        Point.PrevInst = &*std::prev(Inst->getIterator());
+        Point.PrevInst = std::prev(Inst->getIterator());
       } else {
         Point.BB = BB;
       }
@@ -3297,7 +3297,7 @@ class TypePromotionTransaction {
       if (HasPrevInstruction) {
         if (Inst->getParent())
           Inst->removeFromParent();
-        Inst->insertAfter(&*Point.PrevInst);
+        Inst->insertAfter(Point.PrevInst);
       } else {
         BasicBlock::iterator Position = Point.BB->getFirstInsertionPt();
         if (Inst->getParent())
@@ -3317,7 +3317,7 @@ class TypePromotionTransaction {
 
   public:
     /// Move \p Inst before \p Before.
-    InstructionMoveBefore(Instruction *Inst, Instruction *Before)
+    InstructionMoveBefore(Instruction *Inst, BasicBlock::iterator Before)
         : TypePromotionAction(Inst), Position(Inst) {
       LLVM_DEBUG(dbgs() << "Do: move: " << *Inst << "\nbefore: " << *Before
                         << "\n");
diff --git a/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp b/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp
index ce933108b83b12..27f22758d28381 100644
--- a/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp
@@ -342,7 +342,7 @@ class AlignVectors {
   MoveList createLoadGroups(const AddrList &Group) const;
   MoveList createStoreGroups(const AddrList &Group) const;
   bool moveTogether(MoveGroup &Move) const;
-  template <typename T> InstMap cloneBefore(Instruction *To, T &&Insts) const;
+  template <typename T> InstMap cloneBefore(BasicBlock::iterator To, T &&Insts) const;
 
   void realignLoadGroup(IRBuilderBase &Builder, const ByteSpan &VSpan,
                         int ScLen, Value *AlignVal, Value *AlignAddr) const;
@@ -1046,7 +1046,7 @@ auto AlignVectors::moveTogether(MoveGroup &Move) const -> bool {
   if (Move.IsLoad) {
     // Move all the loads (and dependencies) to where the first load is.
     // Clone all deps to before Where, keeping order.
-    Move.Clones = cloneBefore(Where, Move.Deps);
+    Move.Clones = cloneBefore(Where->getIterator(), Move.Deps);
     // Move all main instructions to after Where, keeping order.
     ArrayRef<Instruction *> Main(Move.Main);
     for (Instruction *M : Main) {
@@ -1067,7 +1067,7 @@ auto AlignVectors::moveTogether(MoveGroup &Move) const -> bool {
     // Move all main instructions to before Where, inverting order.
     ArrayRef<Instruction *> Main(Move.Main);
     for (Instruction *M : Main.drop_front(1)) {
-      M->moveBefore(Where);
+      M->moveBefore(Where->getIterator());
       Where = M;
     }
   }
@@ -1076,7 +1076,7 @@ auto AlignVectors::moveTogether(MoveGroup &Move) const -> bool {
 }
 
 template <typename T>
-auto AlignVectors::cloneBefore(Instruction *To, T &&Insts) const -> InstMap {
+auto AlignVectors::cloneBefore(BasicBlock::iterator To, T &&Insts) const -> InstMap {
   InstMap Map;
 
   for (Instruction *I : Insts) {
@@ -1200,10 +1200,10 @@ auto AlignVectors::realignLoadGroup(IRBuilderBase &Builder,
                             VSpan.section(Start, Width).values());
   };
 
-  auto moveBefore = [this](Instruction *In, Instruction *To) {
+  auto moveBefore = [this](BasicBlock::iterator In, BasicBlock::iterator To) {
     // Move In and its upward dependencies to before To.
     assert(In->getParent() == To->getParent());
-    DepList Deps = getUpwardDeps(In, To);
+    DepList Deps = getUpwardDeps(&*In, &*To);
     In->moveBefore(To);
     // DepList is sorted with respect to positions in the basic block.
     InstMap Map = cloneBefore(In, Deps);
@@ -1236,7 +1236,7 @@ auto AlignVectors::realignLoadGroup(IRBuilderBase &Builder,
       // in order to check legality.
       if (auto *Load = dyn_cast<Instruction>(Loads[Index])) {
         if (!HVC.isSafeToMoveBeforeInBB(*Load, BasePos))
-          moveBefore(Load, &*BasePos);
+          moveBefore(Load->getIterator(), BasePos);
       }
       LLVM_DEBUG(dbgs() << "Loads[" << Index << "]:" << *Loads[Index] << '\n');
     }
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index ff5df12c398c56..da387c5276408b 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -1221,8 +1221,8 @@ static void handleNoSuspendCoroutine(coro::Shape &Shape) {
 // SimplifySuspendPoint needs to check that there is no calls between
 // coro_save and coro_suspend, since any of the calls may potentially resume
 // the coroutine and if that is the case we cannot eliminate the suspend point.
-static bool hasCallsInBlockBetween(Instruction *From, Instruction *To) {
-  for (Instruction *I = From; I != To; I = I->getNextNode()) {
+static bool hasCallsInBlockBetween(iterator_range<BasicBlock::iterator> R) {
+  for (Instruction &I : R) {
     // Assume that no intrinsic can resume the coroutine.
     if (isa<IntrinsicInst>(I))
       continue;
@@ -1256,7 +1256,7 @@ static bool hasCallsInBlocksBetween(BasicBlock *SaveBB, BasicBlock *ResDesBB) {
   Set.erase(ResDesBB);
 
   for (auto *BB : Set)
-    if (hasCallsInBlockBetween(BB->getFirstNonPHI(), nullptr))
+    if (hasCallsInBlockBetween({BB->getFirstNonPHIIt(), BB->end()}))
       return true;
 
   return false;
@@ -1265,17 +1265,19 @@ static bool hasCallsInBlocksBetween(BasicBlock *SaveBB, BasicBlock *ResDesBB) {
 static bool hasCallsBetween(Instruction *Save, Instruction *ResumeOrDestroy) {
   auto *SaveBB = Save->getParent();
   auto *ResumeOrDestroyBB = ResumeOrDestroy->getParent();
+  BasicBlock::iterator SaveIt = Save->getIterator();
+  BasicBlock::iterator ResumeOrDestroyIt = ResumeOrDestroy->getIterator();
 
   if (SaveBB == ResumeOrDestroyBB)
-    return hasCallsInBlockBetween(Save->getNextNode(), ResumeOrDestroy);
+    return hasCallsInBlockBetween({std::next(SaveIt), ResumeOrDestroyIt});
 
   // Any calls from Save to the end of the block?
-  if (hasCallsInBlockBetween(Save->getNextNode(), nullptr))
+  if (hasCallsInBlockBetween({std::next(SaveIt), SaveBB->end()}))
     return true;
 
   // Any calls from begging of the block up to ResumeOrDestroy?
-  if (hasCallsInBlockBetween(ResumeOrDestroyBB->getFirstNonPHI(),
-                             ResumeOrDestroy))
+  if (hasCallsInBlockBetween({ResumeOrDestroyBB->getFirstNonPHIIt(),
+                             ResumeOrDestroyIt}))
     return true;
 
   // Any calls in all of the blocks between SaveBB and ResumeOrDestroyBB?
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index e5f3b7f24bca7a..0aa0cbcfc48b3a 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1661,7 +1661,7 @@ void AddressSanitizer::instrumentMaskedLoadOrStore(
   if (Stride)
     Stride = IB.CreateZExtOrTrunc(Stride, IntptrTy);
 
-  SplitBlockAndInsertForEachLane(EVL, LoopInsertBefore,
+  SplitBlockAndInsertForEachLane(EVL, LoopInsertBefore->getIterator(),
                                  [&](IRBuilderBase &IRB, Value *Index) {
     Value *MaskElem = IRB.CreateExtractElement(Mask, Index);
     if (auto *MaskElemC = dyn_cast<ConstantInt>(MaskElem)) {
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 56d3eb10d73e95..10a796e0ce4d41 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -1272,7 +1272,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
       Value *End =
           IRB.CreateUDiv(RoundUp, ConstantInt::get(MS.IntptrTy, kOriginSize));
       auto [InsertPt, Index] =
-          SplitBlockAndInsertSimpleForLoop(End, &*IRB.GetInsertPoint());
+          SplitBlockAndInsertSimpleForLoop(End, IRB.GetInsertPoint());
       IRB.SetInsertPoint(InsertPt);
 
       Value *GEP = IRB.CreateGEP(MS.OriginTy, OriginPtr, Index);
diff --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index 5a9a7ecdc13bfe..f57ec3f57b23b2 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -6138,7 +6138,7 @@ void LSRInstance::ImplementSolution(
     if (!llvm::all_of(BO->uses(),
                       [&](Use &U) {return DT.dominates(IVIncInsertPos, U);}))
       continue;
-    BO->moveBefore(IVIncInsertPos);
+    BO->moveBefore(IVIncInsertPos->getIterator());
     Changed = true;
   }
 
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 78116770009980..91aef9846863da 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -1729,7 +1729,7 @@ void llvm::SplitBlockAndInsertIfThenElse(
 }
 
 std::pair<Instruction*, Value*>
-llvm::SplitBlockAndInsertSimpleForLoop(Value *End, Instruction *SplitBefore) {
+llvm::SplitBlockAndInsertSimpleForLoop(Value *End, BasicBlock::iterator SplitBefore) {
   BasicBlock *LoopPred = SplitBefore->getParent();
   BasicBlock *LoopBody = SplitBlock(SplitBefore->getParent(), SplitBefore);
   BasicBlock *LoopExit = SplitBlock(SplitBefore->getParent(), SplitBefore);
@@ -1752,14 +1752,14 @@ llvm::SplitBlockAndInsertSimpleForLoop(Value *End, Instruction *SplitBefore) {
   IV->addIncoming(ConstantInt::get(Ty, 0), LoopPred);
   IV->addIncoming(IVNext, LoopBody);
 
-  return std::make_pair(LoopBody->getFirstNonPHI(), IV);
+  return std::make_pair(&*LoopBody->getFirstNonPHIIt(), IV);
 }
 
 void llvm::SplitBlockAndInsertForEachLane(ElementCount EC,
-     Type *IndexTy, Instruction *InsertBefore,
+     Type *IndexTy, BasicBlock::iterator InsertBefore,
      std::function<void(IRBuilderBase&, Value*)> Func) {
 
-  IRBuilder<> IRB(InsertBefore);
+  IRBuilder<> IRB(InsertBefore->getParent(), InsertBefore);
 
   if (EC.isScalable()) {
     Value *NumElements = IRB.CreateElementCount(IndexTy, EC);
@@ -1780,10 +1780,10 @@ void llvm::SplitBlockAndInsertForEachLane(ElementCount EC,
 }
 
 void llvm::SplitBlockAndInsertForEachLane(
-    Value *EVL, Instruction *InsertBefore,
+    Value *EVL, BasicBlock::iterator InsertBefore,
     std::function<void(IRBuilderBase &, Value *)> Func) {
 
-  IRBuilder<> IRB(InsertBefore);
+  IRBuilder<> IRB(InsertBefore->getParent(), InsertBefore);
   Type *Ty = EVL->getType();
 
   if (!isa<ConstantInt>(EVL)) {
diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 6a8a468ebcae2b..adc40da07d9670 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -184,14 +184,14 @@ namespace {
 } // end anonymous namespace
 
 static IntrinsicInst *getConvergenceEntry(BasicBlock &BB) {
-  auto *I = BB.getFirstNonPHI();
-  while (I) {
-    if (auto *IntrinsicCall = dyn_cast<ConvergenceControlInst>(I)) {
+  BasicBlock::iterator It = BB.getFirstNonPHIIt();
+  while (It != BB.end()) {
+    if (auto *IntrinsicCall = dyn_cast<ConvergenceControlInst>(It)) {
       if (IntrinsicCall->isEntry()) {
         return IntrinsicCall;
       }
     }
-    I = I->getNextNode();
+    It = std::next(It);
   }
   return nullptr;
 }

Copy link

github-actions bot commented Jan 24, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff c546b5317c518987a5f45dd4c4d25321a955c758 a589eb12e56c1b5fefb382c19bcfded38c13e6d5 --extensions h,cpp -- clang/lib/CodeGen/CodeGenFunction.h llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h llvm/lib/CodeGen/CodeGenPrepare.cpp llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp llvm/lib/Transforms/Coroutines/CoroSplit.cpp llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp llvm/lib/Transforms/Utils/BasicBlockUtils.cpp llvm/lib/Transforms/Utils/InlineFunction.cpp
View the diff from clang-format here.
diff --git a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
index 6faff3d1fd..1bc1e0bb87 100644
--- a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
@@ -539,7 +539,7 @@ inline void SplitBlockAndInsertIfThenElse(Value *Cond, Instruction *SplitBefore,
 /// that \p End is assumed > 0, and thus not checked on entry) at \p
 /// SplitBefore.  Returns the first insert point in the loop body, and the
 /// PHINode for the induction variable (i.e. "i" above).
-std::pair<Instruction*, Value*>
+std::pair<Instruction *, Value *>
 SplitBlockAndInsertSimpleForLoop(Value *End, BasicBlock::iterator SplitBefore);
 
 /// Utility function for performing a given action on each lane of a vector
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 0aa0cbcfc4..e39c42dcbb 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1661,37 +1661,40 @@ void AddressSanitizer::instrumentMaskedLoadOrStore(
   if (Stride)
     Stride = IB.CreateZExtOrTrunc(Stride, IntptrTy);
 
-  SplitBlockAndInsertForEachLane(EVL, LoopInsertBefore->getIterator(),
-                                 [&](IRBuilderBase &IRB, Value *Index) {
-    Value *MaskElem = IRB.CreateExtractElement(Mask, Index);
-    if (auto *MaskElemC = dyn_cast<ConstantInt>(MaskElem)) {
-      if (MaskElemC->isZero())
-        // No check
-        return;
-      // Unconditional check
-    } else {
-      // Conditional check
-      Instruction *ThenTerm = SplitBlockAndInsertIfThen(
-          MaskElem, &*IRB.GetInsertPoint(), false);
-      IRB.SetInsertPoint(ThenTerm);
-    }
+  SplitBlockAndInsertForEachLane(
+      EVL, LoopInsertBefore->getIterator(),
+      [&](IRBuilderBase &IRB, Value *Index) {
+        Value *MaskElem = IRB.CreateExtractElement(Mask, Index);
+        if (auto *MaskElemC = dyn_cast<ConstantInt>(MaskElem)) {
+          if (MaskElemC->isZero())
+            // No check
+            return;
+          // Unconditional check
+        } else {
+          // Conditional check
+          Instruction *ThenTerm = SplitBlockAndInsertIfThen(
+              MaskElem, &*IRB.GetInsertPoint(), false);
+          IRB.SetInsertPoint(ThenTerm);
+        }
 
-    Value *InstrumentedAddress;
-    if (isa<VectorType>(Addr->getType())) {
-      assert(
-          cast<VectorType>(Addr->getType())->getElementType()->isPointerTy() &&
-          "Expected vector of pointer.");
-      InstrumentedAddress = IRB.CreateExtractElement(Addr, Index);
-    } else if (Stride) {
-      Index = IRB.CreateMul(Index, Stride);
-      InstrumentedAddress = IRB.CreatePtrAdd(Addr, Index);
-    } else {
-      InstrumentedAddress = IRB.CreateGEP(VTy, Addr, {Zero, Index});
-    }
-    doInstrumentAddress(Pass, I, &*IRB.GetInsertPoint(), InstrumentedAddress,
-                        Alignment, Granularity, ElemTypeSize, IsWrite,
-                        SizeArgument, UseCalls, Exp, RTCI);
-  });
+        Value *InstrumentedAddress;
+        if (isa<VectorType>(Addr->getType())) {
+          assert(cast<VectorType>(Addr->getType())
+                     ->getElementType()
+                     ->isPointerTy() &&
+                 "Expected vector of pointer.");
+          InstrumentedAddress = IRB.CreateExtractElement(Addr, Index);
+        } else if (Stride) {
+          Index = IRB.CreateMul(Index, Stride);
+          InstrumentedAddress = IRB.CreatePtrAdd(Addr, Index);
+        } else {
+          InstrumentedAddress = IRB.CreateGEP(VTy, Addr, {Zero, Index});
+        }
+        doInstrumentAddress(Pass, I, &*IRB.GetInsertPoint(),
+                            InstrumentedAddress, Alignment, Granularity,
+                            ElemTypeSize, IsWrite, SizeArgument, UseCalls, Exp,
+                            RTCI);
+      });
 }
 
 void AddressSanitizer::instrumentMop(ObjectSizeOffsetVisitor &ObjSizeVis,

Copy link
Contributor

@OCHyams OCHyams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jmorse
Copy link
Member Author

jmorse commented Jan 27, 2025

Ta; fixed some of the clang-format errors that are on lines that I've touched, however I'm leaving the clang-format errors that are adjacent (there are some pre-existing indentation issues too it seems).

@jmorse jmorse merged commit e14962a into llvm:main Jan 27, 2025
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:Hexagon clang:codegen clang Clang issues not falling into any other category compiler-rt:sanitizer coroutines C++20 coroutines llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants