[llvm] r304488 - Reapply "[Cloning] Take another pass at properly cloning debug info"
Keno Fischer via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 1 22:51:03 PDT 2017
Hi Quentin,
if you're duplicating a function that has debug info within the same
module, you'll have to set ModuleLevelChanges to true, because that
now requires global metadata changes (it did before this commit, this
one just makes sure to not generate invalid IR). I suspect
`/*ModuleLevelChanges=*/F->hasSubprogram()` would work for you.
Keno
On Thu, Jun 1, 2017 at 10:56 PM, Quentin Colombet via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> 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
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list