From 5f1ef8aec6932a6fc6125021ebf3ace693c58eed Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Tue, 14 Apr 2015 00:35:42 +0000 Subject: [PATCH] DebugInfo: Move DILocation::computeNewDiscriminators() As documented in PR23200 (and the FIXMEs I've added to the code here), this logic is fairly broken: it modifies the `LLVMContext` in a way that affects other modules and cannot be serialized to assembly/bitcode. For now, move it over to `MDLocation::computeNewDiscriminators()` anyway. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@234825 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/DebugInfo.h | 3 --- include/llvm/IR/DebugInfoMetadata.h | 9 +++++++++ lib/IR/DebugInfo.cpp | 5 ----- lib/IR/DebugInfoMetadata.cpp | 17 +++++++++++++++++ lib/Transforms/Utils/AddDiscriminators.cpp | 8 +++++++- 5 files changed, 33 insertions(+), 9 deletions(-) diff --git a/include/llvm/IR/DebugInfo.h b/include/llvm/IR/DebugInfo.h index 40b21717a0d..6fcf1ee65cd 100644 --- a/include/llvm/IR/DebugInfo.h +++ b/include/llvm/IR/DebugInfo.h @@ -702,9 +702,6 @@ public: StringRef getFilename() const { return get()->getFilename(); } StringRef getDirectory() const { return get()->getDirectory(); } unsigned getDiscriminator() const { return get()->getDiscriminator(); } - - /// \brief Generate a new discriminator value for this location. - unsigned computeNewDiscriminator(LLVMContext &Ctx); }; class DIObjCProperty : public DIDescriptor { diff --git a/include/llvm/IR/DebugInfoMetadata.h b/include/llvm/IR/DebugInfoMetadata.h index 9cb1ac2b8ad..48fc8bbb951 100644 --- a/include/llvm/IR/DebugInfoMetadata.h +++ b/include/llvm/IR/DebugInfoMetadata.h @@ -1209,6 +1209,15 @@ public: /// instructions that are on different basic blocks. inline unsigned getDiscriminator() const; + /// \brief Compute new discriminator in the given context. + /// + /// This modifies the \a LLVMContext that \c this is in to increment the next + /// discriminator for \c this's line/filename combination. + /// + /// FIXME: Delete this. See comments in implementation and at the only call + /// site in \a AddDiscriminators::runOnFunction(). + unsigned computeNewDiscriminator() const; + Metadata *getRawScope() const { return getOperand(0); } Metadata *getRawInlinedAt() const { if (getNumOperands() == 2) diff --git a/lib/IR/DebugInfo.cpp b/lib/IR/DebugInfo.cpp index a2d58fca67a..6bb69bad37a 100644 --- a/lib/IR/DebugInfo.cpp +++ b/lib/IR/DebugInfo.cpp @@ -47,11 +47,6 @@ void DICompileUnit::replaceGlobalVariables(DIArray GlobalVariables) { get()->replaceGlobalVariables(MDGlobalVariableArray(GlobalVariables)); } -unsigned DILocation::computeNewDiscriminator(LLVMContext &Ctx) { - std::pair Key(getFilename().data(), getLineNumber()); - return ++Ctx.pImpl->DiscriminatorTable[Key]; -} - DIVariable llvm::createInlinedVariable(MDNode *DV, MDNode *InlinedScope, LLVMContext &VMContext) { return cast(DV) diff --git a/lib/IR/DebugInfoMetadata.cpp b/lib/IR/DebugInfoMetadata.cpp index ed6206264b0..1ad09dafa4e 100644 --- a/lib/IR/DebugInfoMetadata.cpp +++ b/lib/IR/DebugInfoMetadata.cpp @@ -66,6 +66,23 @@ MDLocation *MDLocation::getImpl(LLVMContext &Context, unsigned Line, Storage, Context.pImpl->MDLocations); } +unsigned MDLocation::computeNewDiscriminator() const { + // FIXME: This seems completely wrong. + // + // 1. If two modules are generated in the same context, then the second + // Module will get different discriminators than it would have if it were + // generated in its own context. + // 2. If this function is called after round-tripping to bitcode instead of + // before, it will give a different (and potentially incorrect!) return. + // + // The discriminator should instead be calculated from local information + // where it's actually needed. This logic should be moved to + // AddDiscriminators::runOnFunction(), where it doesn't pollute the + // LLVMContext. + std::pair Key(getFilename().data(), getLine()); + return ++getContext().pImpl->DiscriminatorTable[Key]; +} + unsigned DebugNode::getFlag(StringRef Flag) { return StringSwitch(Flag) #define HANDLE_DI_FLAG(ID, NAME) .Case("DIFlag" #NAME, Flag##NAME) diff --git a/lib/Transforms/Utils/AddDiscriminators.cpp b/lib/Transforms/Utils/AddDiscriminators.cpp index 4197e35f267..2cbe93e30aa 100644 --- a/lib/Transforms/Utils/AddDiscriminators.cpp +++ b/lib/Transforms/Utils/AddDiscriminators.cpp @@ -195,7 +195,13 @@ bool AddDiscriminators::runOnFunction(Function &F) { StringRef Filename = FirstDIL.getFilename(); DIScope Scope = FirstDIL.getScope(); DIFile File = Builder.createFile(Filename, Scope.getDirectory()); - unsigned Discriminator = FirstDIL.computeNewDiscriminator(Ctx); + + // FIXME: Calculate the discriminator here, based on local information, + // and delete MDLocation::computeNewDiscriminator(). The current + // solution gives different results depending on other modules in the + // same context. All we really need is to discriminate between + // FirstDIL and LastDIL -- a local map would suffice. + unsigned Discriminator = FirstDIL->computeNewDiscriminator(); DILexicalBlockFile NewScope = Builder.createLexicalBlockFile(Scope, File, Discriminator); DILocation NewDIL = -- 2.34.1