[llvm] r304488 - Reapply "[Cloning] Take another pass at properly cloning debug info"
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 1 19:56:37 PDT 2017
Hi Keno,
This breaks some internal builds.
We basically hit this assertion:
Assertion failed: (!MustCloneSP || ModuleLevelChanges), function CloneFunctionInto, file lib/Transforms/Utils/CloneFunction.cpp, line 130.
Our invocation looks like this:
CloneFunctionInto(NewFunc, F, VMap, /*ModuleLevelChanges=*/false, Returns);
Could you advise on how we should fix this?
Cheers,
-Quentin
> On Jun 1, 2017, at 4:02 PM, Keno Fischer via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: kfischer
> Date: Thu Jun 1 18:02:12 2017
> New Revision: 304488
>
> URL: http://llvm.org/viewvc/llvm-project?rev=304488&view=rev
> Log:
> Reapply "[Cloning] Take another pass at properly cloning debug info"
>
> This was rL304226, reverted in 304228 due to a clang assertion failure
> on the build bots. That problem should have been addressed by clang
> commit rL304470.
>
> Modified:
> llvm/trunk/include/llvm/IR/DebugLoc.h
> llvm/trunk/include/llvm/Transforms/Utils/Cloning.h
> llvm/trunk/lib/IR/DebugLoc.cpp
> llvm/trunk/lib/Transforms/Coroutines/CoroSplit.cpp
> llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
> llvm/trunk/unittests/Transforms/Utils/Cloning.cpp
>
> Modified: llvm/trunk/include/llvm/IR/DebugLoc.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugLoc.h?rev=304488&r1=304487&r2=304488&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/DebugLoc.h (original)
> +++ llvm/trunk/include/llvm/IR/DebugLoc.h Thu Jun 1 18:02:12 2017
> @@ -90,12 +90,6 @@ namespace llvm {
> DenseMap<const MDNode *, MDNode *> &Cache,
> bool ReplaceLast = false);
>
> - /// Reparent all debug locations referenced by \c I that belong to \c OrigSP
> - /// to become (possibly indirect) children of \c NewSP.
> - static void reparentDebugInfo(Instruction &I, DISubprogram *OrigSP,
> - DISubprogram *NewSP,
> - DenseMap<const MDNode *, MDNode *> &Cache);
> -
> unsigned getLine() const;
> unsigned getCol() const;
> MDNode *getScope() const;
>
> Modified: llvm/trunk/include/llvm/Transforms/Utils/Cloning.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/Cloning.h?rev=304488&r1=304487&r2=304488&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Transforms/Utils/Cloning.h (original)
> +++ llvm/trunk/include/llvm/Transforms/Utils/Cloning.h Thu Jun 1 18:02:12 2017
> @@ -36,6 +36,7 @@ class BasicBlock;
> class BlockFrequencyInfo;
> class CallInst;
> class CallGraph;
> +class DebugInfoFinder;
> class DominatorTree;
> class Function;
> class Instruction;
> @@ -110,7 +111,8 @@ struct ClonedCodeInfo {
> ///
> BasicBlock *CloneBasicBlock(const BasicBlock *BB, ValueToValueMapTy &VMap,
> const Twine &NameSuffix = "", Function *F = nullptr,
> - ClonedCodeInfo *CodeInfo = nullptr);
> + ClonedCodeInfo *CodeInfo = nullptr,
> + DebugInfoFinder *DIFinder = nullptr);
>
> /// CloneFunction - Return a copy of the specified function and add it to that
> /// function's module. Also, any references specified in the VMap are changed
>
> Modified: llvm/trunk/lib/IR/DebugLoc.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugLoc.cpp?rev=304488&r1=304487&r2=304488&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/DebugLoc.cpp (original)
> +++ llvm/trunk/lib/IR/DebugLoc.cpp Thu Jun 1 18:02:12 2017
> @@ -99,87 +99,6 @@ DebugLoc DebugLoc::appendInlinedAt(Debug
> return Last;
> }
>
> -/// Reparent \c Scope from \c OrigSP to \c NewSP.
> -static DIScope *reparentScope(LLVMContext &Ctx, DIScope *Scope,
> - DISubprogram *OrigSP, DISubprogram *NewSP,
> - DenseMap<const MDNode *, MDNode *> &Cache) {
> - SmallVector<DIScope *, 3> ScopeChain;
> - DIScope *Last = NewSP;
> - DIScope *CurScope = Scope;
> - do {
> - if (auto *SP = dyn_cast<DISubprogram>(CurScope)) {
> - // Don't rewrite this scope chain if it doesn't lead to the replaced SP.
> - if (SP != OrigSP)
> - return Scope;
> - Cache.insert({OrigSP, NewSP});
> - break;
> - }
> - if (auto *Found = Cache[CurScope]) {
> - Last = cast<DIScope>(Found);
> - break;
> - }
> - ScopeChain.push_back(CurScope);
> - } while ((CurScope = CurScope->getScope().resolve()));
> -
> - // Starting from the top, rebuild the nodes to point to the new inlined-at
> - // location (then rebuilding the rest of the chain behind it) and update the
> - // map of already-constructed inlined-at nodes.
> - for (const DIScope *MD : reverse(ScopeChain)) {
> - if (auto *LB = dyn_cast<DILexicalBlock>(MD))
> - Cache[MD] = Last = DILexicalBlock::getDistinct(
> - Ctx, Last, LB->getFile(), LB->getLine(), LB->getColumn());
> - else if (auto *LB = dyn_cast<DILexicalBlockFile>(MD))
> - Cache[MD] = Last = DILexicalBlockFile::getDistinct(
> - Ctx, Last, LB->getFile(), LB->getDiscriminator());
> - else
> - llvm_unreachable("illegal parent scope");
> - }
> - return Last;
> -}
> -
> -void DebugLoc::reparentDebugInfo(Instruction &I, DISubprogram *OrigSP,
> - DISubprogram *NewSP,
> - DenseMap<const MDNode *, MDNode *> &Cache) {
> - auto DL = I.getDebugLoc();
> - if (!OrigSP || !NewSP || OrigSP == NewSP || !DL)
> - return;
> -
> - // Reparent the debug location.
> - auto &Ctx = I.getContext();
> - DILocation *InlinedAt = DL->getInlinedAt();
> - if (InlinedAt) {
> - while (auto *IA = InlinedAt->getInlinedAt())
> - InlinedAt = IA;
> - auto NewScope =
> - reparentScope(Ctx, InlinedAt->getScope(), OrigSP, NewSP, Cache);
> - InlinedAt =
> - DebugLoc::get(InlinedAt->getLine(), InlinedAt->getColumn(), NewScope);
> - }
> - I.setDebugLoc(
> - DebugLoc::get(DL.getLine(), DL.getCol(),
> - reparentScope(Ctx, DL->getScope(), OrigSP, NewSP, Cache),
> - DebugLoc::appendInlinedAt(DL, InlinedAt, Ctx, Cache,
> - ReplaceLastInlinedAt)));
> -
> - // Fix up debug variables to point to NewSP.
> - auto reparentVar = [&](DILocalVariable *Var) {
> - return DILocalVariable::get(
> - Ctx,
> - cast<DILocalScope>(
> - reparentScope(Ctx, Var->getScope(), OrigSP, NewSP, Cache)),
> - Var->getName(), Var->getFile(), Var->getLine(), Var->getType(),
> - Var->getArg(), Var->getFlags(), Var->getAlignInBits());
> - };
> - if (auto *DbgValue = dyn_cast<DbgValueInst>(&I)) {
> - auto *Var = DbgValue->getVariable();
> - I.setOperand(2, MetadataAsValue::get(Ctx, reparentVar(Var)));
> - } else if (auto *DbgDeclare = dyn_cast<DbgDeclareInst>(&I)) {
> - auto *Var = DbgDeclare->getVariable();
> - I.setOperand(1, MetadataAsValue::get(Ctx, reparentVar(Var)));
> - }
> -}
> -
> -
> #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
> LLVM_DUMP_METHOD void DebugLoc::dump() const {
> if (!Loc)
>
> Modified: llvm/trunk/lib/Transforms/Coroutines/CoroSplit.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Coroutines/CoroSplit.cpp?rev=304488&r1=304487&r2=304488&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Coroutines/CoroSplit.cpp (original)
> +++ llvm/trunk/lib/Transforms/Coroutines/CoroSplit.cpp Thu Jun 1 18:02:12 2017
> @@ -228,7 +228,7 @@ static Function *createClone(Function &F
>
> SmallVector<ReturnInst *, 4> Returns;
>
> - CloneFunctionInto(NewF, &F, VMap, /*ModuleLevelChanges=*/false, Returns);
> + CloneFunctionInto(NewF, &F, VMap, /*ModuleLevelChanges=*/true, Returns);
>
> // Remove old returns.
> for (ReturnInst *Return : Returns)
>
> Modified: llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp?rev=304488&r1=304487&r2=304488&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp Thu Jun 1 18:02:12 2017
> @@ -37,10 +37,10 @@
> using namespace llvm;
>
> /// See comments in Cloning.h.
> -BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB,
> - ValueToValueMapTy &VMap,
> +BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB, ValueToValueMapTy &VMap,
> const Twine &NameSuffix, Function *F,
> - ClonedCodeInfo *CodeInfo) {
> + ClonedCodeInfo *CodeInfo,
> + DebugInfoFinder *DIFinder) {
> DenseMap<const MDNode *, MDNode *> Cache;
> BasicBlock *NewBB = BasicBlock::Create(BB->getContext(), "", F);
> if (BB->hasName()) NewBB->setName(BB->getName()+NameSuffix);
> @@ -50,10 +50,11 @@ BasicBlock *llvm::CloneBasicBlock(const
> // Loop over all instructions, and copy them over.
> for (BasicBlock::const_iterator II = BB->begin(), IE = BB->end();
> II != IE; ++II) {
> +
> + if (DIFinder && F->getParent() && II->getDebugLoc())
> + DIFinder->processLocation(*F->getParent(), II->getDebugLoc().get());
> +
> Instruction *NewInst = II->clone();
> - if (F && F->getSubprogram())
> - DebugLoc::reparentDebugInfo(*NewInst, BB->getParent()->getSubprogram(),
> - F->getSubprogram(), Cache);
> if (II->hasName())
> NewInst->setName(II->getName()+NameSuffix);
> NewBB->getInstList().push_back(NewInst);
> @@ -122,31 +123,38 @@ void llvm::CloneFunctionInto(Function *N
> AttributeList::get(NewFunc->getContext(), OldAttrs.getFnAttributes(),
> OldAttrs.getRetAttributes(), NewArgAttrs));
>
> + bool MustCloneSP =
> + OldFunc->getParent() && OldFunc->getParent() == NewFunc->getParent();
> + DISubprogram *SP = OldFunc->getSubprogram();
> + if (SP) {
> + assert(!MustCloneSP || ModuleLevelChanges);
> + // Add mappings for some DebugInfo nodes that we don't want duplicated
> + // even if they're distinct.
> + auto &MD = VMap.MD();
> + MD[SP->getUnit()].reset(SP->getUnit());
> + MD[SP->getType()].reset(SP->getType());
> + MD[SP->getFile()].reset(SP->getFile());
> + // If we're not cloning into the same module, no need to clone the
> + // subprogram
> + if (!MustCloneSP)
> + MD[SP].reset(SP);
> + }
> +
> SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
> OldFunc->getAllMetadata(MDs);
> for (auto MD : MDs) {
> - MDNode *NewMD;
> - bool MustCloneSP =
> - (MD.first == LLVMContext::MD_dbg && OldFunc->getParent() &&
> - OldFunc->getParent() == NewFunc->getParent());
> - if (MustCloneSP) {
> - auto *SP = cast<DISubprogram>(MD.second);
> - NewMD = DISubprogram::getDistinct(
> - NewFunc->getContext(), SP->getScope(), SP->getName(),
> - SP->getLinkageName(), SP->getFile(), SP->getLine(), SP->getType(),
> - SP->isLocalToUnit(), SP->isDefinition(), SP->getScopeLine(),
> - SP->getContainingType(), SP->getVirtuality(), SP->getVirtualIndex(),
> - SP->getThisAdjustment(), SP->getFlags(), SP->isOptimized(),
> - SP->getUnit(), SP->getTemplateParams(), SP->getDeclaration(),
> - SP->getVariables(), SP->getThrownTypes());
> - } else
> - NewMD =
> - MapMetadata(MD.second, VMap,
> - ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges,
> - TypeMapper, Materializer);
> - NewFunc->addMetadata(MD.first, *NewMD);
> + NewFunc->addMetadata(
> + MD.first,
> + *MapMetadata(MD.second, VMap,
> + ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges,
> + TypeMapper, Materializer));
> }
>
> + // When we remap instructions, we want to avoid duplicating inlined
> + // DISubprograms, so record all subprograms we find as we duplicate
> + // instructions and then freeze them in the MD map.
> + DebugInfoFinder DIFinder;
> +
> // Loop over all of the basic blocks in the function, cloning them as
> // appropriate. Note that we save BE this way in order to handle cloning of
> // recursive functions into themselves.
> @@ -156,7 +164,8 @@ void llvm::CloneFunctionInto(Function *N
> const BasicBlock &BB = *BI;
>
> // Create a new basic block and copy instructions into it!
> - BasicBlock *CBB = CloneBasicBlock(&BB, VMap, NameSuffix, NewFunc, CodeInfo);
> + BasicBlock *CBB = CloneBasicBlock(&BB, VMap, NameSuffix, NewFunc, CodeInfo,
> + SP ? &DIFinder : nullptr);
>
> // Add basic block mapping.
> VMap[&BB] = CBB;
> @@ -178,6 +187,12 @@ void llvm::CloneFunctionInto(Function *N
> Returns.push_back(RI);
> }
>
> + for (DISubprogram *ISP : DIFinder.subprograms()) {
> + if (ISP != SP) {
> + VMap.MD()[ISP].reset(ISP);
> + }
> + }
> +
> // Loop over all of the instructions in the function, fixing up operand
> // references as we go. This uses VMap to do all the hard work.
> for (Function::iterator BB =
> @@ -226,7 +241,7 @@ Function *llvm::CloneFunction(Function *
> }
>
> SmallVector<ReturnInst*, 8> Returns; // Ignore returns cloned.
> - CloneFunctionInto(NewF, F, VMap, /*ModuleLevelChanges=*/false, Returns, "",
> + CloneFunctionInto(NewF, F, VMap, F->getSubprogram() != nullptr, Returns, "",
> CodeInfo);
>
> return NewF;
>
> Modified: llvm/trunk/unittests/Transforms/Utils/Cloning.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/Cloning.cpp?rev=304488&r1=304487&r2=304488&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/Transforms/Utils/Cloning.cpp (original)
> +++ llvm/trunk/unittests/Transforms/Utils/Cloning.cpp Thu Jun 1 18:02:12 2017
> @@ -361,7 +361,7 @@ TEST_F(CloneFunc, NewFunctionCreated) {
> // Test that a new subprogram entry was added and is pointing to the new
> // function, while the original subprogram still points to the old one.
> TEST_F(CloneFunc, Subprogram) {
> - EXPECT_FALSE(verifyModule(*M));
> + EXPECT_FALSE(verifyModule(*M, &errs()));
> EXPECT_EQ(3U, Finder->subprogram_count());
> EXPECT_NE(NewFunc->getSubprogram(), OldFunc->getSubprogram());
> }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170601/c518ba48/attachment.html>
More information about the llvm-commits
mailing list