Skip to content

Conversation

@jmorse
Copy link
Member

@jmorse jmorse commented Jul 15, 2025

With the advent of intrinsic-less debug-info, we no longer need to scatter calls to getPrevNonDebugInstruction around the codebase. Remove most of them -- however there are one or two that have the "SkipPseudoOp" flag turned on, indicating that those call-sites are intended to skip more than just debug-info intrinsics.

To avoid making a functional change, I've renamed the method to getPrevNonPseudoOpInstruction and left it in the positions that request that behaviour using the SkipPseudoOp flag.

@jmorse jmorse requested review from OCHyams, SLTozer and WenleiHe July 15, 2025 14:34
@jmorse jmorse requested a review from nikic as a code owner July 15, 2025 14:34
@llvmbot llvmbot added llvm:codegen llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes compiler-rt:sanitizer flang:openmp llvm:ir llvm:transforms clang:openmp OpenMP related changes to Clang labels Jul 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 15, 2025

@llvm/pr-subscribers-compiler-rt-sanitizer
@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-llvm-ir

Author: Jeremy Morse (jmorse)

Changes

With the advent of intrinsic-less debug-info, we no longer need to scatter calls to getPrevNonDebugInstruction around the codebase. Remove most of them -- however there are one or two that have the "SkipPseudoOp" flag turned on, indicating that those call-sites are intended to skip more than just debug-info intrinsics.

To avoid making a functional change, I've renamed the method to getPrevNonPseudoOpInstruction and left it in the positions that request that behaviour using the SkipPseudoOp flag.


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

13 Files Affected:

  • (modified) llvm/include/llvm/IR/Instruction.h (+5-7)
  • (modified) llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h (+2-2)
  • (modified) llvm/lib/CodeGen/CodeGenPrepare.cpp (+2-2)
  • (modified) llvm/lib/CodeGen/StackProtector.cpp (+1-1)
  • (modified) llvm/lib/IR/Instruction.cpp (+2-2)
  • (modified) llvm/lib/Transforms/IPO/OpenMPOpt.cpp (+1-1)
  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+1-1)
  • (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+1-1)
  • (modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+1-1)
  • (modified) llvm/lib/Transforms/Scalar/GVN.cpp (+1-1)
  • (modified) llvm/lib/Transforms/Scalar/GVNSink.cpp (+1-1)
  • (modified) llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp (+1-1)
  • (modified) llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp (+2-2)
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index ef382a9168f24..8e2ef436a04a0 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -909,15 +909,13 @@ class Instruction : public User,
             SkipPseudoOp));
   }
 
-  /// Return a pointer to the previous non-debug instruction in the same basic
-  /// block as 'this', or nullptr if no such instruction exists. Skip any pseudo
-  /// operations if \c SkipPseudoOp is true.
+  /// Return a pointer to the previous non-pseudo-op instruction in the same
+  /// basic block as 'this', or nullptr if no such instruction exists.
   LLVM_ABI const Instruction *
-  getPrevNonDebugInstruction(bool SkipPseudoOp = false) const;
-  Instruction *getPrevNonDebugInstruction(bool SkipPseudoOp = false) {
+  getPrevNonPseudoOpInstruction() const;
+  Instruction *getPrevNonPseudoOpInstruction() {
     return const_cast<Instruction *>(
-        static_cast<const Instruction *>(this)->getPrevNonDebugInstruction(
-            SkipPseudoOp));
+       static_cast<const Instruction *>(this)->getPrevNonPseudoOpInstruction());
   }
 
   /// Create a copy of 'this' instruction that is identical in all ways except
diff --git a/llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h b/llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h
index 1b6309c7fb1a4..836374554128b 100644
--- a/llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h
+++ b/llvm/include/llvm/Transforms/Utils/LockstepReverseIterator.h
@@ -61,7 +61,7 @@ class LockstepReverseIterator
     }
     Insts.clear();
     for (BasicBlock *BB : Blocks) {
-      Instruction *Prev = BB->getTerminator()->getPrevNonDebugInstruction();
+      Instruction *Prev = BB->getTerminator()->getPrevNode();
       if (!Prev) {
         // Block wasn't big enough - only contained a terminator.
         if constexpr (EarlyFailure) {
@@ -108,7 +108,7 @@ class LockstepReverseIterator
       return *this;
     SmallVector<Instruction *, 4> NewInsts;
     for (Instruction *Inst : Insts) {
-      Instruction *Prev = Inst->getPrevNonDebugInstruction();
+      Instruction *Prev = Inst->getPrevNode();
       if (!Prev) {
         if constexpr (!EarlyFailure) {
           this->ActiveBlocks.remove(Inst->getParent());
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index 9bbb89e37865d..27715aad461e1 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -3015,7 +3015,7 @@ bool CodeGenPrepare::dupRetToEnableTailCallOpts(BasicBlock *BB,
         //   %phi = phi ptr [ %0, %bb0 ], [ %2, %entry ]
         if (PredBB && PredBB->getSingleSuccessor() == BB)
           CI = dyn_cast_or_null<CallInst>(
-              PredBB->getTerminator()->getPrevNonDebugInstruction(true));
+              PredBB->getTerminator()->getPrevNonPseudoOpInstruction());
 
         if (CI && CI->use_empty() &&
             isIntrinsicOrLFToBeTailCalled(TLInfo, CI) &&
@@ -3032,7 +3032,7 @@ bool CodeGenPrepare::dupRetToEnableTailCallOpts(BasicBlock *BB,
     for (BasicBlock *Pred : predecessors(BB)) {
       if (!VisitedBBs.insert(Pred).second)
         continue;
-      if (Instruction *I = Pred->rbegin()->getPrevNonDebugInstruction(true)) {
+      if (Instruction *I = Pred->rbegin()->getPrevNonPseudoOpInstruction()) {
         CallInst *CI = dyn_cast<CallInst>(I);
         if (CI && CI->use_empty() && TLI->mayBeEmittedAsTailCall(CI) &&
             attributesPermitTailCall(F, CI, RetI, *TLI)) {
diff --git a/llvm/lib/CodeGen/StackProtector.cpp b/llvm/lib/CodeGen/StackProtector.cpp
index 3ec70083b7043..9cc9af88c5e4f 100644
--- a/llvm/lib/CodeGen/StackProtector.cpp
+++ b/llvm/lib/CodeGen/StackProtector.cpp
@@ -626,7 +626,7 @@ bool InsertStackProtectors(const TargetMachine *TM, Function *F,
 
     // If we're instrumenting a block with a tail call, the check has to be
     // inserted before the call rather than between it and the return.
-    Instruction *Prev = CheckLoc->getPrevNonDebugInstruction();
+    Instruction *Prev = CheckLoc->getPrevNode();
     if (auto *CI = dyn_cast_if_present<CallInst>(Prev))
       if (CI->isTailCall() && isInTailCallPosition(*CI, *TM))
         CheckLoc = Prev;
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index cbf39b8adf1b2..83f45d2d16fd6 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -1244,9 +1244,9 @@ Instruction::getNextNonDebugInstruction(bool SkipPseudoOp) const {
 }
 
 const Instruction *
-Instruction::getPrevNonDebugInstruction(bool SkipPseudoOp) const {
+Instruction::getPrevNonPseudoOpInstruction() const {
   for (const Instruction *I = getPrevNode(); I; I = I->getPrevNode())
-    if (!isa<DbgInfoIntrinsic>(I) && !(SkipPseudoOp && isa<PseudoProbeInst>(I)))
+    if (!isa<PseudoProbeInst>(I))
       return I;
   return nullptr;
 }
diff --git a/llvm/lib/Transforms/IPO/OpenMPOpt.cpp b/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
index dd7ae7e66e350..1ca89b5937ba4 100644
--- a/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
+++ b/llvm/lib/Transforms/IPO/OpenMPOpt.cpp
@@ -2875,7 +2875,7 @@ struct AAExecutionDomainFunction : public AAExecutionDomain {
       if (It->getSecond().IsReachedFromAlignedBarrierOnly)
         break;
       return false;
-    } while ((CurI = CurI->getPrevNonDebugInstruction()));
+    } while ((CurI = CurI->getPrevNode()));
 
     // Delayed decision on the forward pass to allow aligned barrier detection
     // in the backwards traversal.
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index e521c9d7001ac..a222fef3f70bd 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3933,7 +3933,7 @@ Instruction *InstCombinerImpl::visitFenceInst(FenceInst &FI) {
   if (NFI && isIdenticalOrStrongerFence(NFI, &FI))
     return eraseInstFromFunction(FI);
 
-  if (auto *PFI = dyn_cast_or_null<FenceInst>(FI.getPrevNonDebugInstruction()))
+  if (auto *PFI = dyn_cast_or_null<FenceInst>(FI.getPrevNode()))
     if (isIdenticalOrStrongerFence(PFI, &FI))
       return eraseInstFromFunction(FI);
   return nullptr;
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 91a1b61ddc483..b587d76465803 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3890,7 +3890,7 @@ bool InstCombinerImpl::removeInstructionsBeforeUnreachable(Instruction &I) {
   // This includes instructions like stores and "llvm.assume" that may not get
   // removed by simple dead code elimination.
   bool Changed = false;
-  while (Instruction *Prev = I.getPrevNonDebugInstruction()) {
+  while (Instruction *Prev = I.getPrevNode()) {
     // While we theoretically can erase EH, that would result in a block that
     // used to start with an EH no longer starting with EH, which is invalid.
     // To make it valid, we'd need to fixup predecessors to no longer refer to
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 840a5e3f31dfd..18806a93b76cd 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3424,7 +3424,7 @@ static void findStoresToUninstrumentedArgAllocas(
           isa<Argument>(cast<CastInst>(Val)->getOperand(0)) &&
           // Check that the cast appears directly before the store. Otherwise
           // moving the cast before InsBefore may break the IR.
-          Val == It->getPrevNonDebugInstruction();
+          Val == It->getPrevNode();
       bool IsArgInit = IsDirectArgInit || IsArgInitViaCast;
       if (!IsArgInit)
         continue;
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index d9d05c3e8cc49..8bff458f88bb9 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -1310,7 +1310,7 @@ static Value *findDominatingValue(const MemoryLocation &Loc, Type *LoadTy,
   BatchAAResults BatchAA(*AA);
   for (BasicBlock *BB = FromBB; BB; BB = BB->getSinglePredecessor())
     for (auto *Inst = BB == FromBB ? From : BB->getTerminator();
-         Inst != nullptr; Inst = Inst->getPrevNonDebugInstruction()) {
+         Inst != nullptr; Inst = Inst->getPrevNode()) {
       // Stop the search if limit is reached.
       if (++NumVisitedInsts > MaxNumVisitedInsts)
         return nullptr;
diff --git a/llvm/lib/Transforms/Scalar/GVNSink.cpp b/llvm/lib/Transforms/Scalar/GVNSink.cpp
index 2058df33ea331..a5fc0b4c6904d 100644
--- a/llvm/lib/Transforms/Scalar/GVNSink.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNSink.cpp
@@ -799,7 +799,7 @@ void GVNSink::sinkLastInstruction(ArrayRef<BasicBlock *> Blocks,
                                   BasicBlock *BBEnd) {
   SmallVector<Instruction *, 4> Insts;
   for (BasicBlock *BB : Blocks)
-    Insts.push_back(BB->getTerminator()->getPrevNonDebugInstruction());
+    Insts.push_back(BB->getTerminator()->getPrevNode());
   Instruction *I0 = Insts.front();
 
   SmallVector<Value *, 4> NewOperands;
diff --git a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
index a09303bb4469f..609edc9312489 100644
--- a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
+++ b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
@@ -195,7 +195,7 @@ static bool tailMergeBlocksWithSimilarFunctionTerminators(Function &F,
     // of the value computed by experimental_deoptimize.
     // I.e., we can not change `ret` to `br` for this block.
     if (auto *CI =
-            dyn_cast_or_null<CallInst>(Term->getPrevNonDebugInstruction())) {
+            dyn_cast_or_null<CallInst>(Term->getPrevNode())) {
       if (Function *F = CI->getCalledFunction())
         if (Intrinsic::ID ID = F->getIntrinsicID())
           if (ID == Intrinsic::experimental_deoptimize)
diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index d019ade03ec45..04089797f7b54 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -2737,7 +2737,7 @@ TEST_P(OpenMPIRBuilderTestWithParams, DynamicWorkShareLoop) {
   EXPECT_EQ(OrigStride->getValue(), 1);
 
   CallInst *FiniCall = dyn_cast<CallInst>(
-      &*(LatchBlock->getTerminator()->getPrevNonDebugInstruction(true)));
+      &*(LatchBlock->getTerminator()->getPrevNonPseudoOpInstruction()));
   EXPECT_EQ(FiniCall, nullptr);
 
   // The original loop iterator should only be used in the condition, in the
@@ -2841,7 +2841,7 @@ TEST_F(OpenMPIRBuilderTest, DynamicWorkShareLoopOrdered) {
             static_cast<uint64_t>(OMPScheduleType::OrderedStaticChunked));
 
   CallInst *FiniCall = dyn_cast<CallInst>(
-      &*(LatchBlock->getTerminator()->getPrevNonDebugInstruction(true)));
+      &*(LatchBlock->getTerminator()->getPrevNonPseudoOpInstruction()));
   ASSERT_NE(FiniCall, nullptr);
   EXPECT_EQ(FiniCall->getCalledFunction()->getName(),
             "__kmpc_dispatch_fini_4u");

@jmorse
Copy link
Member Author

jmorse commented Jul 15, 2025

I agree and have removed; I suppose we should ping @WenleiHe and wait a bit again, as pseudo-probe stuff is generally unclear to me.

@nikic
Copy link
Contributor

nikic commented Jul 15, 2025

This needs a rebase so the CI can run.

jmorse added 2 commits July 16, 2025 09:54
With the advent of intrinsic-less debug-info, we no longer need to scatter
calls to getPrevNonDebugInstruction around the codebase. Remove most of
them -- however there are one or two that have the "SkipPseudoOp" flag
turned on, indicating that those call-sites are intended to skip more than
just debug-info intrinsics.

To avoid making a functional change, I've renamed the method to
getPrevNonPseudoOpInstruction and left it in the positions that request
that behaviour using the SkipPseudoOp flag.
@jmorse jmorse force-pushed the ddd-remove-skipping-backwards branch from fb895ed to 8bafeb4 Compare July 16, 2025 08:56
@github-actions
Copy link

github-actions bot commented Jul 16, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@nikic nikic 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 Jul 16, 2025

I've got a bunch of other patches to go in with this, I suppose we can revert back if pseudo-probes really turns out to need this behaviour.

@jmorse jmorse merged commit 5b8c15c into llvm:main Jul 16, 2025
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:openmp OpenMP related changes to Clang compiler-rt:sanitizer flang:openmp llvm:codegen llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:ir llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants