[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 23:42:54 PDT 2017
Thanks, I’ll give it a shot.
> On Jun 1, 2017, at 10:51 PM, Keno Fischer <keno at juliacomputing.com> wrote:
>
> 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