[llvm] r234825 - DebugInfo: Move DILocation::computeNewDiscriminators()

[Duncan P. N. Exon Smith]

Author: dexonsmith
Date: Mon Apr 13 19:35:42 2015
New Revision: 234825

URL: http://llvm.org/viewvc/llvm-project?rev=234825&view=rev
Log:
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.

Modified:
    llvm/trunk/include/llvm/IR/DebugInfo.h
    llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
    llvm/trunk/lib/IR/DebugInfo.cpp
    llvm/trunk/lib/IR/DebugInfoMetadata.cpp
    llvm/trunk/lib/Transforms/Utils/AddDiscriminators.cpp

Modified: llvm/trunk/include/llvm/IR/DebugInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfo.h?rev=234825&r1=234824&r2=234825&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/DebugInfo.h (original)
+++ llvm/trunk/include/llvm/IR/DebugInfo.h Mon Apr 13 19:35:42 2015
@@ -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 {

Modified: llvm/trunk/include/llvm/IR/DebugInfoMetadata.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DebugInfoMetadata.h?rev=234825&r1=234824&r2=234825&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/DebugInfoMetadata.h (original)
+++ llvm/trunk/include/llvm/IR/DebugInfoMetadata.h Mon Apr 13 19:35:42 2015
@@ -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)

Modified: llvm/trunk/lib/IR/DebugInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfo.cpp?rev=234825&r1=234824&r2=234825&view=diff
==============================================================================
--- llvm/trunk/lib/IR/DebugInfo.cpp (original)
+++ llvm/trunk/lib/IR/DebugInfo.cpp Mon Apr 13 19:35:42 2015
@@ -47,11 +47,6 @@ void DICompileUnit::replaceGlobalVariabl
   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)

Modified: llvm/trunk/lib/IR/DebugInfoMetadata.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfoMetadata.cpp?rev=234825&r1=234824&r2=234825&view=diff
==============================================================================
--- llvm/trunk/lib/IR/DebugInfoMetadata.cpp (original)
+++ llvm/trunk/lib/IR/DebugInfoMetadata.cpp Mon Apr 13 19:35:42 2015
@@ -66,6 +66,23 @@ MDLocation *MDLocation::getImpl(LLVMCont
                    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)

Modified: llvm/trunk/lib/Transforms/Utils/AddDiscriminators.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/AddDiscriminators.cpp?rev=234825&r1=234824&r2=234825&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/AddDiscriminators.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/AddDiscriminators.cpp Mon Apr 13 19:35:42 2015
@@ -195,7 +195,13 @@ bool AddDiscriminators::runOnFunction(Fu
         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 =