From: Duncan P. N. Exon Smith <dexonsmith@apple.com>
Date: Tue, 14 Apr 2015 00:35:42 +0000 (+0000)
Subject: DebugInfo: Move DILocation::computeNewDiscriminators()
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=5f1ef8aec6932a6fc6125021ebf3ace693c58eed;p=oota-llvm.git

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
---

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<const char *, unsigned> Key(getFilename().data(), getLineNumber());
-  return ++Ctx.pImpl->DiscriminatorTable[Key];
-}
-
 DIVariable llvm::createInlinedVariable(MDNode *DV, MDNode *InlinedScope,
                                        LLVMContext &VMContext) {
   return cast<MDLocalVariable>(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<const char *, unsigned> Key(getFilename().data(), getLine());
+  return ++getContext().pImpl->DiscriminatorTable[Key];
+}
+
 unsigned DebugNode::getFlag(StringRef Flag) {
   return StringSwitch<unsigned>(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 =