From b0c8ec6d141b86afad2ce21d52199e77cef5b757 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Tue, 15 Jul 2025 15:23:00 +0100 Subject: [PATCH 1/3] [DebugInfo] Remove getPrevNonDebugInstruction 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. --- llvm/include/llvm/IR/Instruction.h | 7 +++---- .../llvm/Transforms/Utils/LockstepReverseIterator.h | 4 ++-- llvm/lib/CodeGen/CodeGenPrepare.cpp | 4 ++-- llvm/lib/CodeGen/StackProtector.cpp | 2 +- llvm/lib/IR/Instruction.cpp | 4 ++-- llvm/lib/Transforms/IPO/OpenMPOpt.cpp | 2 +- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp | 2 +- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 2 +- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 2 +- llvm/lib/Transforms/Scalar/GVN.cpp | 2 +- llvm/lib/Transforms/Scalar/GVNSink.cpp | 2 +- llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp | 2 +- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp | 4 ++-- 13 files changed, 19 insertions(+), 20 deletions(-) diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h index c317a06753970..2def6dc945724 100644 --- a/llvm/include/llvm/IR/Instruction.h +++ b/llvm/include/llvm/IR/Instruction.h @@ -902,11 +902,10 @@ class Instruction : public User, /// block as 'this', or nullptr if no such instruction exists. Skip any pseudo /// operations if \c SkipPseudoOp is true. LLVM_ABI const Instruction * - getPrevNonDebugInstruction(bool SkipPseudoOp = false) const; - Instruction *getPrevNonDebugInstruction(bool SkipPseudoOp = false) { + getPrevNonPseudoOpInstruction() const; + Instruction *getPrevNonPseudoOpInstruction() { return const_cast( - static_cast(this)->getPrevNonDebugInstruction( - SkipPseudoOp)); + static_cast(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 cd525a9710103..5b92b33a10ea0 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 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 70a9788c76e1f..006e882af47e3 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( - 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(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(Prev)) if (CI->isTailCall() && isInTailCallPosition(*CI, *TM)) CheckLoc = Prev; diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp index c6dca727e0e89..a49481db6af3d 100644 --- a/llvm/lib/IR/Instruction.cpp +++ b/llvm/lib/IR/Instruction.cpp @@ -1236,9 +1236,9 @@ bool Instruction::isDebugOrPseudoInst() const { } const Instruction * -Instruction::getPrevNonDebugInstruction(bool SkipPseudoOp) const { +Instruction::getPrevNonPseudoOpInstruction() const { for (const Instruction *I = getPrevNode(); I; I = I->getPrevNode()) - if (!isa(I) && !(SkipPseudoOp && isa(I))) + if (!isa(I)) return I; return nullptr; } diff --git a/llvm/lib/Transforms/IPO/OpenMPOpt.cpp b/llvm/lib/Transforms/IPO/OpenMPOpt.cpp index 5de2285c2d2e3..5e2247f2a88d0 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 73293bb5f4a0e..3321435a6fecb 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(FI.getPrevNonDebugInstruction())) + if (auto *PFI = dyn_cast_or_null(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 dfbe4f8172066..5957940add577 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -3424,7 +3424,7 @@ static void findStoresToUninstrumentedArgAllocas( isa(cast(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 Blocks, BasicBlock *BBEnd) { SmallVector Insts; for (BasicBlock *BB : Blocks) - Insts.push_back(BB->getTerminator()->getPrevNonDebugInstruction()); + Insts.push_back(BB->getTerminator()->getPrevNode()); Instruction *I0 = Insts.front(); SmallVector 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(Term->getPrevNonDebugInstruction())) { + dyn_cast_or_null(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 55dc8a79fd9ab..55422942f470c 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( - &*(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(OMPScheduleType::OrderedStaticChunked)); CallInst *FiniCall = dyn_cast( - &*(LatchBlock->getTerminator()->getPrevNonDebugInstruction(true))); + &*(LatchBlock->getTerminator()->getPrevNonPseudoOpInstruction())); ASSERT_NE(FiniCall, nullptr); EXPECT_EQ(FiniCall->getCalledFunction()->getName(), "__kmpc_dispatch_fini_4u"); From 8bafeb41d5f027771f3a082264f73775c677bbdb Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Tue, 15 Jul 2025 16:54:47 +0100 Subject: [PATCH 2/3] Remove getPrev... entirely. --- llvm/include/llvm/IR/Instruction.h | 10 ---------- llvm/lib/CodeGen/CodeGenPrepare.cpp | 4 ++-- llvm/lib/IR/Instruction.cpp | 8 -------- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp | 4 ++-- 4 files changed, 4 insertions(+), 22 deletions(-) diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h index 2def6dc945724..5d25804a684ac 100644 --- a/llvm/include/llvm/IR/Instruction.h +++ b/llvm/include/llvm/IR/Instruction.h @@ -898,16 +898,6 @@ class Instruction : public User, /// Return true if the instruction is a DbgInfoIntrinsic or PseudoProbeInst. LLVM_ABI bool isDebugOrPseudoInst() const LLVM_READONLY; - /// 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. - LLVM_ABI const Instruction * - getPrevNonPseudoOpInstruction() const; - Instruction *getPrevNonPseudoOpInstruction() { - return const_cast( - static_cast(this)->getPrevNonPseudoOpInstruction()); - } - /// Create a copy of 'this' instruction that is identical in all ways except /// the following: /// * The instruction has no parent diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp index 006e882af47e3..d9d41f1d72e35 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( - PredBB->getTerminator()->getPrevNonPseudoOpInstruction()); + PredBB->getTerminator()->getPrevNode()); 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()->getPrevNonPseudoOpInstruction()) { + if (Instruction *I = Pred->rbegin()->getPrevNode()) { CallInst *CI = dyn_cast(I); if (CI && CI->use_empty() && TLI->mayBeEmittedAsTailCall(CI) && attributesPermitTailCall(F, CI, RetI, *TLI)) { diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp index a49481db6af3d..763cc1832b794 100644 --- a/llvm/lib/IR/Instruction.cpp +++ b/llvm/lib/IR/Instruction.cpp @@ -1235,14 +1235,6 @@ bool Instruction::isDebugOrPseudoInst() const { return isa(this) || isa(this); } -const Instruction * -Instruction::getPrevNonPseudoOpInstruction() const { - for (const Instruction *I = getPrevNode(); I; I = I->getPrevNode()) - if (!isa(I)) - return I; - return nullptr; -} - const DebugLoc &Instruction::getStableDebugLoc() const { return getDebugLoc(); } diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp index 55422942f470c..c519030c710a1 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( - &*(LatchBlock->getTerminator()->getPrevNonPseudoOpInstruction())); + &*(LatchBlock->getTerminator()->getPrevNode())); 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(OMPScheduleType::OrderedStaticChunked)); CallInst *FiniCall = dyn_cast( - &*(LatchBlock->getTerminator()->getPrevNonPseudoOpInstruction())); + &*(LatchBlock->getTerminator()->getPrevNode())); ASSERT_NE(FiniCall, nullptr); EXPECT_EQ(FiniCall->getCalledFunction()->getName(), "__kmpc_dispatch_fini_4u"); From 219e7de1f5f768270004fea426f640daf054e6ff Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Wed, 16 Jul 2025 11:36:01 +0100 Subject: [PATCH 3/3] clang-format --- llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp | 3 +-- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp index 609edc9312489..60e5df08c6efd 100644 --- a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp +++ b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp @@ -194,8 +194,7 @@ static bool tailMergeBlocksWithSimilarFunctionTerminators(Function &F, // Calls to experimental_deoptimize must be followed by a return // 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(Term->getPrevNode())) { + if (auto *CI = dyn_cast_or_null(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 c519030c710a1..d6b578aa8ffd1 100644 --- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp +++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp @@ -2736,8 +2736,8 @@ TEST_P(OpenMPIRBuilderTestWithParams, DynamicWorkShareLoop) { EXPECT_EQ(OrigUpperBound->getValue(), 21); EXPECT_EQ(OrigStride->getValue(), 1); - CallInst *FiniCall = dyn_cast( - &*(LatchBlock->getTerminator()->getPrevNode())); + CallInst *FiniCall = + dyn_cast(&*(LatchBlock->getTerminator()->getPrevNode())); EXPECT_EQ(FiniCall, nullptr); // The original loop iterator should only be used in the condition, in the @@ -2840,8 +2840,8 @@ TEST_F(OpenMPIRBuilderTest, DynamicWorkShareLoopOrdered) { EXPECT_EQ(SchedVal->getValue(), static_cast(OMPScheduleType::OrderedStaticChunked)); - CallInst *FiniCall = dyn_cast( - &*(LatchBlock->getTerminator()->getPrevNode())); + CallInst *FiniCall = + dyn_cast(&*(LatchBlock->getTerminator()->getPrevNode())); ASSERT_NE(FiniCall, nullptr); EXPECT_EQ(FiniCall->getCalledFunction()->getName(), "__kmpc_dispatch_fini_4u");