From 862f55356667b92b92d72a9993e7215c70bb94b0 Mon Sep 17 00:00:00 2001 From: OCHyams Date: Mon, 4 Dec 2023 16:49:34 +0000 Subject: [PATCH 1/3] [RemoveDIs] Update Coroutine passes to handle DPValues As part of the RemoveDIs project, transitioning to non-instruction debug info, all debug intrinsic handling code needs to be duplicated to handle DPValues. --try-experimental-debuginfo-iterators enables the new debug mode in tests if the CMake option has been enabled. --- llvm/lib/IR/BasicBlock.cpp | 2 + llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 182 ++++++++++++++---- llvm/lib/Transforms/Coroutines/CoroInternal.h | 15 +- llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 28 ++- .../Transforms/Coroutines/coro-debug-O2.ll | 1 + .../Coroutines/coro-debug-coro-frame.ll | 1 + ...coro-debug-dbg.values-not_used_in_frame.ll | 1 + .../Coroutines/coro-debug-dbg.values.ll | 1 + .../Coroutines/coro-debug-frame-variable.ll | 1 + .../coro-debug-spill-dbg.declare.ll | 1 + llvm/test/Transforms/Coroutines/coro-debug.ll | 1 + .../Transforms/Coroutines/coro-split-dbg.ll | 1 + .../Transforms/Coroutines/swift-async-dbg.ll | 7 + 13 files changed, 190 insertions(+), 52 deletions(-) diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp index f364c56a42c52..7a84c298552f5 100644 --- a/llvm/lib/IR/BasicBlock.cpp +++ b/llvm/lib/IR/BasicBlock.cpp @@ -1088,6 +1088,8 @@ void BasicBlock::insertDPValueBefore(DPValue *DPV, // shouldn't be generated at times when there's no terminator. assert(Where != end()); assert(Where->getParent() == this); + if (!Where->DbgMarker) + createMarker(Where); bool InsertAtHead = Where.getHeadBit(); Where->DbgMarker->insertDPValue(DPV, InsertAtHead); } diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp index 6b85de15947c2..3f201b5d910b4 100644 --- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp @@ -964,12 +964,17 @@ static void cacheDIVar(FrameDataInfo &FrameData, continue; SmallVector DDIs; - findDbgDeclares(DDIs, V); - auto *I = llvm::find_if(DDIs, [](DbgDeclareInst *DDI) { - return DDI->getExpression()->getNumElements() == 0; - }); - if (I != DDIs.end()) - DIVarCache.insert({V, (*I)->getVariable()}); + SmallVector DPVs; + findDbgDeclares(DDIs, V, &DPVs); + auto CacheIt = [&DIVarCache, V](auto &Container) { + auto *I = llvm::find_if(Container, [](auto *DDI) { + return DDI->getExpression()->getNumElements() == 0; + }); + if (I != Container.end()) + DIVarCache.insert({V, (*I)->getVariable()}); + }; + CacheIt(DDIs); + CacheIt(DPVs); } } @@ -1121,15 +1126,25 @@ static void buildFrameDebugInfo(Function &F, coro::Shape &Shape, "Coroutine with switch ABI should own Promise alloca"); SmallVector DIs; - findDbgDeclares(DIs, PromiseAlloca); - if (DIs.empty()) + SmallVector DPVs; + findDbgDeclares(DIs, PromiseAlloca, &DPVs); + + DILocalVariable *PromiseDIVariable = nullptr; + DILocation *DILoc = nullptr; + if (!DIs.empty()) { + DbgDeclareInst *PromiseDDI = DIs.front(); + PromiseDIVariable = PromiseDDI->getVariable(); + DILoc = PromiseDDI->getDebugLoc().get(); + } else if (!DPVs.empty()) { + DPValue *PromiseDPV = DPVs.front(); + PromiseDIVariable = PromiseDPV->getVariable(); + DILoc = PromiseDPV->getDebugLoc().get(); + } else { return; + } - DbgDeclareInst *PromiseDDI = DIs.front(); - DILocalVariable *PromiseDIVariable = PromiseDDI->getVariable(); DILocalScope *PromiseDIScope = PromiseDIVariable->getScope(); DIFile *DFile = PromiseDIScope->getFile(); - DILocation *DILoc = PromiseDDI->getDebugLoc().get(); unsigned LineNum = PromiseDIVariable->getLine(); DICompositeType *FrameDITy = DBuilder.createStructType( @@ -1243,7 +1258,7 @@ static void buildFrameDebugInfo(Function &F, coro::Shape &Shape, auto *FrameDIVar = DBuilder.createAutoVariable(PromiseDIScope, "__coro_frame", DFile, LineNum, FrameDITy, true, DINode::FlagArtificial); - assert(FrameDIVar->isValidLocationForIntrinsic(PromiseDDI->getDebugLoc())); + assert(FrameDIVar->isValidLocationForIntrinsic(DILoc)); // Subprogram would have ContainedNodes field which records the debug // variables it contained. So we need to add __coro_frame to the @@ -1261,9 +1276,17 @@ static void buildFrameDebugInfo(Function &F, coro::Shape &Shape, 7, (MDTuple::get(F.getContext(), RetainedNodesVec))); } - DBuilder.insertDeclare(Shape.FramePtr, FrameDIVar, - DBuilder.createExpression(), DILoc, - Shape.getInsertPtAfterFramePtr()); + if (UseNewDbgInfoFormat) { + DPValue *NewDPV = new DPValue(ValueAsMetadata::get(Shape.FramePtr), + FrameDIVar, DBuilder.createExpression(), + DILoc, DPValue::LocationType::Declare); + BasicBlock::iterator It = Shape.getInsertPtAfterFramePtr(); + It->getParent()->insertDPValueBefore(NewDPV, It); + } else { + DBuilder.insertDeclare(Shape.FramePtr, FrameDIVar, + DBuilder.createExpression(), DILoc, + &*Shape.getInsertPtAfterFramePtr()); + } } // Build a struct that will keep state for an active coroutine. @@ -1771,7 +1794,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) { if (auto *Arg = dyn_cast(Def)) { // For arguments, we will place the store instruction right after // the coroutine frame pointer instruction, i.e. coro.begin. - InsertPt = Shape.getInsertPtAfterFramePtr()->getIterator(); + InsertPt = Shape.getInsertPtAfterFramePtr(); // If we're spilling an Argument, make sure we clear 'nocapture' // from the coroutine function. @@ -1788,7 +1811,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) { if (!DT.dominates(CB, I)) { // If it is not dominated by CoroBegin, then spill should be // inserted immediately after CoroFrame is computed. - InsertPt = Shape.getInsertPtAfterFramePtr()->getIterator(); + InsertPt = Shape.getInsertPtAfterFramePtr(); } else if (auto *II = dyn_cast(I)) { // If we are spilling the result of the invoke instruction, split // the normal edge and insert the spill in the new block. @@ -1843,7 +1866,8 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) { SpillAlignment, E.first->getName() + Twine(".reload")); SmallVector DIs; - findDbgDeclares(DIs, Def); + SmallVector DPVs; + findDbgDeclares(DIs, Def, &DPVs); // Try best to find dbg.declare. If the spill is a temp, there may not // be a direct dbg.declare. Walk up the load chain to find one from an // alias. @@ -1858,24 +1882,36 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) { if (!isa(CurDef)) break; DIs.clear(); - findDbgDeclares(DIs, CurDef); + DPVs.clear(); + findDbgDeclares(DIs, CurDef, &DPVs); } } - for (DbgDeclareInst *DDI : DIs) { + auto SalvageOne = [&](auto *DDI) { bool AllowUnresolved = false; // This dbg.declare is preserved for all coro-split function // fragments. It will be unreachable in the main function, and // processed by coro::salvageDebugInfo() by CoroCloner. - DIBuilder(*CurrentBlock->getParent()->getParent(), AllowUnresolved) - .insertDeclare(CurrentReload, DDI->getVariable(), - DDI->getExpression(), DDI->getDebugLoc(), - &*Builder.GetInsertPoint()); + if (UseNewDbgInfoFormat) { + DPValue *NewDPV = + new DPValue(ValueAsMetadata::get(CurrentReload), + DDI->getVariable(), DDI->getExpression(), + DDI->getDebugLoc(), DPValue::LocationType::Declare); + Builder.GetInsertPoint()->getParent()->insertDPValueBefore( + NewDPV, Builder.GetInsertPoint()); + } else { + DIBuilder(*CurrentBlock->getParent()->getParent(), AllowUnresolved) + .insertDeclare(CurrentReload, DDI->getVariable(), + DDI->getExpression(), DDI->getDebugLoc(), + &*Builder.GetInsertPoint()); + } // This dbg.declare is for the main function entry point. It // will be deleted in all coro-split functions. coro::salvageDebugInfo(ArgToAllocaMap, DDI, Shape.OptimizeFrame, false /*UseEntryValue*/); - } + }; + for_each(DIs, SalvageOne); + for_each(DPVs, SalvageOne); } // If we have a single edge PHINode, remove it and replace it with a @@ -1893,6 +1929,10 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) { // Replace all uses of CurrentValue in the current instruction with // reload. U->replaceUsesOfWith(Def, CurrentReload); + // Instructions are added to Def's user list if the attached + // debug records use Def. Update those now. + for (auto &DPV: U->getDbgValueRange()) + DPV.replaceVariableLocationOp(Def, CurrentReload, true); } } @@ -1943,9 +1983,12 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) { G->setName(Alloca->getName() + Twine(".reload.addr")); SmallVector DIs; - findDbgUsers(DIs, Alloca); + SmallVector DPValues; + findDbgUsers(DIs, Alloca, &DPValues); for (auto *DVI : DIs) DVI->replaceUsesOfWith(Alloca, G); + for (auto *DPV : DPValues) + DPV->replaceVariableLocationOp(Alloca, G); for (Instruction *I : UsersToUpdate) { // It is meaningless to retain the lifetime intrinsics refer for the @@ -1959,7 +2002,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) { I->replaceUsesOfWith(Alloca, G); } } - Builder.SetInsertPoint(Shape.getInsertPtAfterFramePtr()); + Builder.SetInsertPoint(&*Shape.getInsertPtAfterFramePtr()); for (const auto &A : FrameData.Allocas) { AllocaInst *Alloca = A.Alloca; if (A.MayWriteBeforeCoroBegin) { @@ -2020,7 +2063,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) { isa(Inst); }); if (HasAccessingPromiseBeforeCB) { - Builder.SetInsertPoint(Shape.getInsertPtAfterFramePtr()); + Builder.SetInsertPoint(&*Shape.getInsertPtAfterFramePtr()); auto *G = GetFramePointer(PA); auto *Value = Builder.CreateLoad(PA->getAllocatedType(), PA); Builder.CreateStore(Value, G); @@ -2802,21 +2845,16 @@ static void collectFrameAlloca(AllocaInst *AI, coro::Shape &Shape, Visitor.getMayWriteBeforeCoroBegin()); } -void coro::salvageDebugInfo( - SmallDenseMap &ArgToAllocaMap, - DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool UseEntryValue) { - Function *F = DVI->getFunction(); +static std::optional> +salvageDebugInfoImpl(SmallDenseMap &ArgToAllocaMap, + bool OptimizeFrame, bool UseEntryValue, Function *F, + Value *Storage, DIExpression *Expr, + bool SkipOutermostLoad) { IRBuilder<> Builder(F->getContext()); auto InsertPt = F->getEntryBlock().getFirstInsertionPt(); while (isa(InsertPt)) ++InsertPt; Builder.SetInsertPoint(&F->getEntryBlock(), InsertPt); - DIExpression *Expr = DVI->getExpression(); - // Follow the pointer arithmetic all the way to the incoming - // function argument and convert into a DIExpression. - bool SkipOutermostLoad = !isa(DVI); - Value *Storage = DVI->getVariableLocationOp(0); - Value *OriginalStorage = Storage; while (auto *Inst = dyn_cast_or_null(Storage)) { if (auto *LdInst = dyn_cast(Inst)) { @@ -2848,7 +2886,7 @@ void coro::salvageDebugInfo( SkipOutermostLoad = false; } if (!Storage) - return; + return std::nullopt; auto *StorageAsArg = dyn_cast(Storage); const bool IsSwiftAsyncArg = @@ -2884,6 +2922,28 @@ void coro::salvageDebugInfo( Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore); } + return {{Storage, Expr}}; +} + +void coro::salvageDebugInfo( + SmallDenseMap &ArgToAllocaMap, + DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool UseEntryValue) { + + Function *F = DVI->getFunction(); + // Follow the pointer arithmetic all the way to the incoming + // function argument and convert into a DIExpression. + bool SkipOutermostLoad = !isa(DVI); + Value *OriginalStorage = DVI->getVariableLocationOp(0); + + auto SalvagedInfo = ::salvageDebugInfoImpl( + ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage, + DVI->getExpression(), SkipOutermostLoad); + if (!SalvagedInfo) + return; + + Value *Storage = SalvagedInfo->first; + DIExpression *Expr = SalvagedInfo->second; + DVI->replaceVariableLocationOp(OriginalStorage, Storage); DVI->setExpression(Expr); // We only hoist dbg.declare today since it doesn't make sense to hoist @@ -2900,6 +2960,43 @@ void coro::salvageDebugInfo( } } +void coro::salvageDebugInfo( + SmallDenseMap &ArgToAllocaMap, DPValue *DPV, + bool OptimizeFrame, bool UseEntryValue) { + + Function *F = DPV->getFunction(); + // Follow the pointer arithmetic all the way to the incoming + // function argument and convert into a DIExpression. + bool SkipOutermostLoad = DPV->getType() == DPValue::LocationType::Declare; + Value *OriginalStorage = DPV->getVariableLocationOp(0); + + auto SalvagedInfo = ::salvageDebugInfoImpl( + ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage, + DPV->getExpression(), SkipOutermostLoad); + if (!SalvagedInfo) + return; + + Value *Storage = SalvagedInfo->first; + DIExpression *Expr = SalvagedInfo->second; + + DPV->replaceVariableLocationOp(OriginalStorage, Storage); + DPV->setExpression(Expr); + // We only hoist dbg.declare today since it doesn't make sense to hoist + // dbg.value since it does not have the same function wide guarantees that + // dbg.declare does. + if (DPV->getType() == DPValue::LocationType::Declare) { + std::optional InsertPt; + if (auto *I = dyn_cast(Storage)) + InsertPt = I->getInsertionPointAfterDef(); + else if (isa(Storage)) + InsertPt = F->getEntryBlock().begin(); + if (InsertPt) { + DPV->removeFromParent(); + (*InsertPt)->getParent()->insertDPValueBefore(DPV, *InsertPt); + } + } +} + static void doRematerializations( Function &F, SuspendCrossingInfo &Checker, const std::function &MaterializableCallback) { @@ -3087,10 +3184,15 @@ void coro::buildCoroutineFrame( for (auto &Iter : FrameData.Spills) { auto *V = Iter.first; SmallVector DVIs; - findDbgValues(DVIs, V); + SmallVector DPVs; + findDbgValues(DVIs, V, &DPVs); for (DbgValueInst *DVI : DVIs) if (Checker.isDefinitionAcrossSuspend(*V, DVI)) FrameData.Spills[V].push_back(DVI); + // Add the instructions which carry debug info that is in the frame. + for (DPValue *DPV : DPVs) + if (Checker.isDefinitionAcrossSuspend(*V, DPV->Marker->MarkedInstr)) + FrameData.Spills[V].push_back(DPV->Marker->MarkedInstr); } LLVM_DEBUG(dumpSpills("Spills", FrameData.Spills)); diff --git a/llvm/lib/Transforms/Coroutines/CoroInternal.h b/llvm/lib/Transforms/Coroutines/CoroInternal.h index 0856c4925cc5d..1022f7a2979f5 100644 --- a/llvm/lib/Transforms/Coroutines/CoroInternal.h +++ b/llvm/lib/Transforms/Coroutines/CoroInternal.h @@ -33,6 +33,10 @@ void replaceCoroFree(CoroIdInst *CoroId, bool Elide); void salvageDebugInfo( SmallDenseMap &ArgToAllocaMap, DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool IsEntryPoint); +/// See overload. +void salvageDebugInfo( + SmallDenseMap &ArgToAllocaMap, DPValue *DPV, + bool OptimizeFrame, bool UseEntryValue); // Keeps data and helper functions for lowering coroutine intrinsics. struct LowererBase { @@ -240,10 +244,13 @@ struct LLVM_LIBRARY_VISIBILITY Shape { return nullptr; } - Instruction *getInsertPtAfterFramePtr() const { - if (auto *I = dyn_cast(FramePtr)) - return I->getNextNode(); - return &cast(FramePtr)->getParent()->getEntryBlock().front(); + BasicBlock::iterator getInsertPtAfterFramePtr() const { + if (auto *I = dyn_cast(FramePtr)) { + BasicBlock::iterator It = std::next(I->getIterator()); + It.setHeadBit(true); // Copy pre-RemoveDIs behaviour. + return It; + } + return cast(FramePtr)->getParent()->getEntryBlock().begin(); } /// Allocate memory according to the rules of the active lowering. diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp index 244580f503d5b..504aa912ac266 100644 --- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp @@ -726,11 +726,14 @@ static void replaceSwiftErrorOps(Function &F, coro::Shape &Shape, /// Returns all DbgVariableIntrinsic in F. static SmallVector -collectDbgVariableIntrinsics(Function &F) { +collectDbgVariableIntrinsics(Function &F, SmallVector &DPValues) { SmallVector Intrinsics; - for (auto &I : instructions(F)) + for (auto &I : instructions(F)) { + for (DPValue &DPV : I.getDbgValueRange()) + DPValues.push_back(&DPV); if (auto *DVI = dyn_cast(&I)) Intrinsics.push_back(DVI); + } return Intrinsics; } @@ -739,8 +742,9 @@ void CoroCloner::replaceSwiftErrorOps() { } void CoroCloner::salvageDebugInfo() { + SmallVector DPValues; SmallVector Worklist = - collectDbgVariableIntrinsics(*NewF); + collectDbgVariableIntrinsics(*NewF, DPValues); SmallDenseMap ArgToAllocaMap; // Only 64-bit ABIs have a register we can refer to with the entry value. @@ -749,6 +753,9 @@ void CoroCloner::salvageDebugInfo() { for (DbgVariableIntrinsic *DVI : Worklist) coro::salvageDebugInfo(ArgToAllocaMap, DVI, Shape.OptimizeFrame, UseEntryValue); + for (DPValue *DPV : DPValues) + coro::salvageDebugInfo(ArgToAllocaMap, DPV, Shape.OptimizeFrame, + UseEntryValue); // Remove all salvaged dbg.declare intrinsics that became // either unreachable or stale due to the CoroSplit transformation. @@ -757,7 +764,7 @@ void CoroCloner::salvageDebugInfo() { return !isPotentiallyReachable(&NewF->getEntryBlock(), BB, nullptr, &DomTree); }; - for (DbgVariableIntrinsic *DVI : Worklist) { + auto RemoveOne = [&](auto *DVI) { if (IsUnreachableBlock(DVI->getParent())) DVI->eraseFromParent(); else if (isa_and_nonnull(DVI->getVariableLocationOp(0))) { @@ -770,7 +777,9 @@ void CoroCloner::salvageDebugInfo() { if (!Uses) DVI->eraseFromParent(); } - } + }; + for_each(Worklist, RemoveOne); + for_each(DPValues, RemoveOne); } void CoroCloner::replaceEntryBlock() { @@ -1243,7 +1252,7 @@ static void updateCoroFrame(coro::Shape &Shape, Function *ResumeFn, Function *DestroyFn, Function *CleanupFn) { assert(Shape.ABI == coro::ABI::Switch); - IRBuilder<> Builder(Shape.getInsertPtAfterFramePtr()); + IRBuilder<> Builder(&*Shape.getInsertPtAfterFramePtr()); auto *ResumeAddr = Builder.CreateStructGEP( Shape.FrameTy, Shape.FramePtr, coro::Shape::SwitchFieldIndex::Resume, @@ -2039,10 +2048,13 @@ splitCoroutine(Function &F, SmallVectorImpl &Clones, // original function. The Cloner has already salvaged debug info in the new // coroutine funclets. SmallDenseMap ArgToAllocaMap; - for (auto *DDI : collectDbgVariableIntrinsics(F)) + SmallVector DPValues; + for (auto *DDI : collectDbgVariableIntrinsics(F, DPValues)) coro::salvageDebugInfo(ArgToAllocaMap, DDI, Shape.OptimizeFrame, false /*UseEntryValue*/); - + for (DPValue *DPV : DPValues) + coro::salvageDebugInfo(ArgToAllocaMap, DPV, Shape.OptimizeFrame, + false /*UseEntryValue*/); return Shape; } diff --git a/llvm/test/Transforms/Coroutines/coro-debug-O2.ll b/llvm/test/Transforms/Coroutines/coro-debug-O2.ll index a668bd1a52196..3f9af30a92e34 100644 --- a/llvm/test/Transforms/Coroutines/coro-debug-O2.ll +++ b/llvm/test/Transforms/Coroutines/coro-debug-O2.ll @@ -1,4 +1,5 @@ ; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split),function(sroa)' -S | FileCheck %s +; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='module(coro-early),cgscc(coro-split),function(sroa)' -S | FileCheck %s ; Checks whether the dbg.declare for `__promise` remains valid under O2. diff --git a/llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll b/llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll index 7f5679e4d522f..2978f85be2385 100644 --- a/llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll +++ b/llvm/test/Transforms/Coroutines/coro-debug-coro-frame.ll @@ -1,4 +1,5 @@ ; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s +; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s ; Checks whether the dbg.declare for `__coro_frame` are created. diff --git a/llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll b/llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll index 1b9a1bd63a2e3..79793dc293d0d 100644 --- a/llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll +++ b/llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll @@ -1,5 +1,6 @@ ; Tests whether resume function would remain dbg.value infomation if corresponding values are not used in the frame. ; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s +; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s ; ; This file is based on coro-debug-frame-variable.ll. ; CHECK: define internal fastcc void @f.resume(ptr noundef nonnull align 16 dereferenceable(80) %begin) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]] diff --git a/llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll b/llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll index 54cdde8ae4ac0..47b2ddafcfc65 100644 --- a/llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll +++ b/llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll @@ -1,5 +1,6 @@ ; Tests whether resume function would remain dbg.value infomation. ; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s +; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s ; ; This file is based on coro-debug-frame-variable.ll. ; CHECK-LABEL: define void @f( diff --git a/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll b/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll index 37b4126ce3730..19a89fefd5269 100644 --- a/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll +++ b/llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll @@ -1,4 +1,5 @@ ; RUN: opt < %s -passes='default' -S | FileCheck %s +; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='default' -S | FileCheck %s ; Define a function 'f' that resembles the Clang frontend's output for the ; following C++ coroutine: diff --git a/llvm/test/Transforms/Coroutines/coro-debug-spill-dbg.declare.ll b/llvm/test/Transforms/Coroutines/coro-debug-spill-dbg.declare.ll index e7a271a96ead1..c943ea5ca22ec 100644 --- a/llvm/test/Transforms/Coroutines/coro-debug-spill-dbg.declare.ll +++ b/llvm/test/Transforms/Coroutines/coro-debug-spill-dbg.declare.ll @@ -1,6 +1,7 @@ ; Test spilling a temp generates dbg.declare in resume/destroy/cleanup functions. ; ; RUN: opt < %s -passes='cgscc(coro-split)' -S | FileCheck %s +; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='cgscc(coro-split)' -S | FileCheck %s ; ; The test case simulates a coroutine method in a class. ; diff --git a/llvm/test/Transforms/Coroutines/coro-debug.ll b/llvm/test/Transforms/Coroutines/coro-debug.ll index 064693c80ad23..4792825f4ce08 100644 --- a/llvm/test/Transforms/Coroutines/coro-debug.ll +++ b/llvm/test/Transforms/Coroutines/coro-debug.ll @@ -1,5 +1,6 @@ ; Tests that debug information is sane after coro-split ; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s +; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s source_filename = "simple-repro.c" target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" diff --git a/llvm/test/Transforms/Coroutines/coro-split-dbg.ll b/llvm/test/Transforms/Coroutines/coro-split-dbg.ll index 184d4a564ab72..7d95be308a5f4 100644 --- a/llvm/test/Transforms/Coroutines/coro-split-dbg.ll +++ b/llvm/test/Transforms/Coroutines/coro-split-dbg.ll @@ -1,6 +1,7 @@ ; Make sure that coro-split correctly deals with debug information. ; The test here is simply that it does not result in bad IR that will crash opt. ; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -disable-output +; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -disable-output source_filename = "coro.c" target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-unknown-linux-gnu" diff --git a/llvm/test/Transforms/Coroutines/swift-async-dbg.ll b/llvm/test/Transforms/Coroutines/swift-async-dbg.ll index af7d816c12861..74edf7a3f3a54 100644 --- a/llvm/test/Transforms/Coroutines/swift-async-dbg.ll +++ b/llvm/test/Transforms/Coroutines/swift-async-dbg.ll @@ -2,6 +2,13 @@ ; RUN: opt -mtriple='x86_64' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s ; RUN: opt -mtriple='i386-' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s --check-prefix=NOENTRY ; RUN: opt -mtriple='armv7-' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s --check-prefix=NOENTRY + +;; Replicate those tests with non-instruction debug markers. +; RUN: opt --try-experimental-debuginfo-iterators -mtriple='arm64-' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s +; RUN: opt --try-experimental-debuginfo-iterators -mtriple='x86_64' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s +; RUN: opt --try-experimental-debuginfo-iterators -mtriple='i386-' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s --check-prefix=NOENTRY +; RUN: opt --try-experimental-debuginfo-iterators -mtriple='armv7-' %s -S -passes='module(coro-early),cgscc(coro-split,simplifycfg)' -o - | FileCheck %s --check-prefix=NOENTRY + ; NOENTRY-NOT: OP_llvm_entry_value target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128" From e63eb59a49699692bddc61e5ceee8c93bca541ce Mon Sep 17 00:00:00 2001 From: OCHyams Date: Wed, 6 Dec 2023 10:51:45 +0000 Subject: [PATCH 2/3] coro: address feedback --- llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 48 +++++++++---------- llvm/lib/Transforms/Coroutines/CoroInternal.h | 5 +- llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 23 +++++---- 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp index 3f201b5d910b4..a1ecb7a998327 100644 --- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp @@ -1907,7 +1907,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) { } // This dbg.declare is for the main function entry point. It // will be deleted in all coro-split functions. - coro::salvageDebugInfo(ArgToAllocaMap, DDI, Shape.OptimizeFrame, + coro::salvageDebugInfo(ArgToAllocaMap, *DDI, Shape.OptimizeFrame, false /*UseEntryValue*/); }; for_each(DIs, SalvageOne); @@ -2845,7 +2845,7 @@ static void collectFrameAlloca(AllocaInst *AI, coro::Shape &Shape, Visitor.getMayWriteBeforeCoroBegin()); } -static std::optional> +static std::optional> salvageDebugInfoImpl(SmallDenseMap &ArgToAllocaMap, bool OptimizeFrame, bool UseEntryValue, Function *F, Value *Storage, DIExpression *Expr, @@ -2922,30 +2922,30 @@ salvageDebugInfoImpl(SmallDenseMap &ArgToAllocaMap, Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore); } - return {{Storage, Expr}}; + return {{*Storage, *Expr}}; } void coro::salvageDebugInfo( SmallDenseMap &ArgToAllocaMap, - DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool UseEntryValue) { + DbgVariableIntrinsic &DVI, bool OptimizeFrame, bool UseEntryValue) { - Function *F = DVI->getFunction(); + Function *F = DVI.getFunction(); // Follow the pointer arithmetic all the way to the incoming // function argument and convert into a DIExpression. bool SkipOutermostLoad = !isa(DVI); - Value *OriginalStorage = DVI->getVariableLocationOp(0); + Value *OriginalStorage = DVI.getVariableLocationOp(0); auto SalvagedInfo = ::salvageDebugInfoImpl( ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage, - DVI->getExpression(), SkipOutermostLoad); + DVI.getExpression(), SkipOutermostLoad); if (!SalvagedInfo) return; - Value *Storage = SalvagedInfo->first; - DIExpression *Expr = SalvagedInfo->second; + Value *Storage = &SalvagedInfo->first; + DIExpression *Expr = &SalvagedInfo->second; - DVI->replaceVariableLocationOp(OriginalStorage, Storage); - DVI->setExpression(Expr); + DVI.replaceVariableLocationOp(OriginalStorage, Storage); + DVI.setExpression(Expr); // We only hoist dbg.declare today since it doesn't make sense to hoist // dbg.value since it does not have the same function wide guarantees that // dbg.declare does. @@ -2956,43 +2956,43 @@ void coro::salvageDebugInfo( else if (isa(Storage)) InsertPt = F->getEntryBlock().begin(); if (InsertPt) - DVI->moveBefore(*(*InsertPt)->getParent(), *InsertPt); + DVI.moveBefore(*(*InsertPt)->getParent(), *InsertPt); } } void coro::salvageDebugInfo( - SmallDenseMap &ArgToAllocaMap, DPValue *DPV, + SmallDenseMap &ArgToAllocaMap, DPValue &DPV, bool OptimizeFrame, bool UseEntryValue) { - Function *F = DPV->getFunction(); + Function *F = DPV.getFunction(); // Follow the pointer arithmetic all the way to the incoming // function argument and convert into a DIExpression. - bool SkipOutermostLoad = DPV->getType() == DPValue::LocationType::Declare; - Value *OriginalStorage = DPV->getVariableLocationOp(0); + bool SkipOutermostLoad = DPV.getType() == DPValue::LocationType::Declare; + Value *OriginalStorage = DPV.getVariableLocationOp(0); auto SalvagedInfo = ::salvageDebugInfoImpl( ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage, - DPV->getExpression(), SkipOutermostLoad); + DPV.getExpression(), SkipOutermostLoad); if (!SalvagedInfo) return; - Value *Storage = SalvagedInfo->first; - DIExpression *Expr = SalvagedInfo->second; + Value *Storage = &SalvagedInfo->first; + DIExpression *Expr = &SalvagedInfo->second; - DPV->replaceVariableLocationOp(OriginalStorage, Storage); - DPV->setExpression(Expr); + DPV.replaceVariableLocationOp(OriginalStorage, Storage); + DPV.setExpression(Expr); // We only hoist dbg.declare today since it doesn't make sense to hoist // dbg.value since it does not have the same function wide guarantees that // dbg.declare does. - if (DPV->getType() == DPValue::LocationType::Declare) { + if (DPV.getType() == DPValue::LocationType::Declare) { std::optional InsertPt; if (auto *I = dyn_cast(Storage)) InsertPt = I->getInsertionPointAfterDef(); else if (isa(Storage)) InsertPt = F->getEntryBlock().begin(); if (InsertPt) { - DPV->removeFromParent(); - (*InsertPt)->getParent()->insertDPValueBefore(DPV, *InsertPt); + DPV.removeFromParent(); + (*InsertPt)->getParent()->insertDPValueBefore(&DPV, *InsertPt); } } } diff --git a/llvm/lib/Transforms/Coroutines/CoroInternal.h b/llvm/lib/Transforms/Coroutines/CoroInternal.h index 1022f7a2979f5..fb16a4090689b 100644 --- a/llvm/lib/Transforms/Coroutines/CoroInternal.h +++ b/llvm/lib/Transforms/Coroutines/CoroInternal.h @@ -32,10 +32,9 @@ void replaceCoroFree(CoroIdInst *CoroId, bool Elide); /// OptimizeFrame is false. void salvageDebugInfo( SmallDenseMap &ArgToAllocaMap, - DbgVariableIntrinsic *DVI, bool OptimizeFrame, bool IsEntryPoint); -/// See overload. + DbgVariableIntrinsic &DVI, bool OptimizeFrame, bool IsEntryPoint); void salvageDebugInfo( - SmallDenseMap &ArgToAllocaMap, DPValue *DPV, + SmallDenseMap &ArgToAllocaMap, DPValue &DPV, bool OptimizeFrame, bool UseEntryValue); // Keeps data and helper functions for lowering coroutine intrinsics. diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp index 504aa912ac266..7758b52abc204 100644 --- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp @@ -725,16 +725,17 @@ static void replaceSwiftErrorOps(Function &F, coro::Shape &Shape, } /// Returns all DbgVariableIntrinsic in F. -static SmallVector -collectDbgVariableIntrinsics(Function &F, SmallVector &DPValues) { +static std::pair, SmallVector> +collectDbgVariableIntrinsics(Function &F) { SmallVector Intrinsics; + SmallVector DPValues; for (auto &I : instructions(F)) { for (DPValue &DPV : I.getDbgValueRange()) DPValues.push_back(&DPV); if (auto *DVI = dyn_cast(&I)) Intrinsics.push_back(DVI); } - return Intrinsics; + return {Intrinsics, DPValues}; } void CoroCloner::replaceSwiftErrorOps() { @@ -742,19 +743,17 @@ void CoroCloner::replaceSwiftErrorOps() { } void CoroCloner::salvageDebugInfo() { - SmallVector DPValues; - SmallVector Worklist = - collectDbgVariableIntrinsics(*NewF, DPValues); + auto [Worklist, DPValues] = collectDbgVariableIntrinsics(*NewF); SmallDenseMap ArgToAllocaMap; // Only 64-bit ABIs have a register we can refer to with the entry value. bool UseEntryValue = llvm::Triple(OrigF.getParent()->getTargetTriple()).isArch64Bit(); for (DbgVariableIntrinsic *DVI : Worklist) - coro::salvageDebugInfo(ArgToAllocaMap, DVI, Shape.OptimizeFrame, + coro::salvageDebugInfo(ArgToAllocaMap, *DVI, Shape.OptimizeFrame, UseEntryValue); for (DPValue *DPV : DPValues) - coro::salvageDebugInfo(ArgToAllocaMap, DPV, Shape.OptimizeFrame, + coro::salvageDebugInfo(ArgToAllocaMap, *DPV, Shape.OptimizeFrame, UseEntryValue); // Remove all salvaged dbg.declare intrinsics that became @@ -2048,12 +2047,12 @@ splitCoroutine(Function &F, SmallVectorImpl &Clones, // original function. The Cloner has already salvaged debug info in the new // coroutine funclets. SmallDenseMap ArgToAllocaMap; - SmallVector DPValues; - for (auto *DDI : collectDbgVariableIntrinsics(F, DPValues)) - coro::salvageDebugInfo(ArgToAllocaMap, DDI, Shape.OptimizeFrame, + auto [DbgInsts, DPValues] = collectDbgVariableIntrinsics(F); + for (auto *DDI : DbgInsts) + coro::salvageDebugInfo(ArgToAllocaMap, *DDI, Shape.OptimizeFrame, false /*UseEntryValue*/); for (DPValue *DPV : DPValues) - coro::salvageDebugInfo(ArgToAllocaMap, DPV, Shape.OptimizeFrame, + coro::salvageDebugInfo(ArgToAllocaMap, *DPV, Shape.OptimizeFrame, false /*UseEntryValue*/); return Shape; } From 6e03e019f3e126950ea66b73e14285861c7e8c3d Mon Sep 17 00:00:00 2001 From: OCHyams Date: Wed, 13 Dec 2023 11:29:36 +0000 Subject: [PATCH 3/3] format --- llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp index a1ecb7a998327..f37b4dc938d30 100644 --- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp @@ -1931,7 +1931,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) { U->replaceUsesOfWith(Def, CurrentReload); // Instructions are added to Def's user list if the attached // debug records use Def. Update those now. - for (auto &DPV: U->getDbgValueRange()) + for (auto &DPV : U->getDbgValueRange()) DPV.replaceVariableLocationOp(Def, CurrentReload, true); } }