<div dir="ltr">I'm sure it's probably obvious, but you've not mentioned it here - why is the "right place" the function, and not the module?</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 26, 2016 at 4:23 PM, Dehao Chen via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">danielcdh created this revision.<br>
danielcdh added reviewers: dnovillo, davidxl.<br>
danielcdh added a subscriber: llvm-commits.<br>
<br>
Now discriminator is assigned per-function instead of per-module.<br>
<br>
<a href="http://reviews.llvm.org/D17664" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17664</a><br>
<br>
Files:<br>
  include/llvm/IR/DebugInfoMetadata.h<br>
  lib/IR/DebugInfoMetadata.cpp<br>
  lib/Transforms/Utils/AddDiscriminators.cpp<br>
<br>
Index: lib/Transforms/Utils/AddDiscriminators.cpp<br>
===================================================================<br>
--- lib/Transforms/Utils/AddDiscriminators.cpp<br>
+++ lib/Transforms/Utils/AddDiscriminators.cpp<br>
@@ -173,8 +173,10 @@<br>
   typedef std::pair<StringRef, unsigned> Location;<br>
   typedef DenseMap<const BasicBlock *, Metadata *> BBScopeMap;<br>
   typedef DenseMap<Location, BBScopeMap> LocationBBMap;<br>
+  typedef DenseMap<Location, unsigned> LocationDiscriminatorMap;<br>
<br>
   LocationBBMap LBM;<br>
+  LocationDiscriminatorMap LDM;<br>
<br>
   // Traverse all instructions in the function. If the source line location<br>
   // of the instruction appears in other basic block, assign a new<br>
@@ -199,8 +201,7 @@<br>
         auto *Scope = DIL->getScope();<br>
         auto *File =<br>
             Builder.createFile(DIL->getFilename(), Scope->getDirectory());<br>
-        NewScope = Builder.createLexicalBlockFile(<br>
-            Scope, File, DIL->computeNewDiscriminator());<br>
+        NewScope = Builder.createLexicalBlockFile(Scope, File, ++LDM[L]);<br>
       }<br>
       I.setDebugLoc(DILocation::get(Ctx, DIL->getLine(), DIL->getColumn(),<br>
                                     NewScope, DIL->getInlinedAt()));<br>
@@ -230,8 +231,10 @@<br>
           auto *Scope = FirstDIL->getScope();<br>
           auto *File = Builder.createFile(FirstDIL->getFilename(),<br>
                                           Scope->getDirectory());<br>
-          auto *NewScope = Builder.createLexicalBlockFile(<br>
-              Scope, File, FirstDIL->computeNewDiscriminator());<br>
+          Location L =<br>
+              std::make_pair(FirstDIL->getFilename(), FirstDIL->getLine());<br>
+          auto *NewScope =<br>
+              Builder.createLexicalBlockFile(Scope, File, ++LDM[L]);<br>
           Current->setDebugLoc(DILocation::get(<br>
               Ctx, CurrentDIL->getLine(), CurrentDIL->getColumn(), NewScope,<br>
               CurrentDIL->getInlinedAt()));<br>
Index: lib/IR/DebugInfoMetadata.cpp<br>
===================================================================<br>
--- lib/IR/DebugInfoMetadata.cpp<br>
+++ lib/IR/DebugInfoMetadata.cpp<br>
@@ -66,23 +66,6 @@<br>
                    Storage, Context.pImpl->DILocations);<br>
 }<br>
<br>
-unsigned DILocation::computeNewDiscriminator() const {<br>
-  // FIXME: This seems completely wrong.<br>
-  //<br>
-  //  1. If two modules are generated in the same context, then the second<br>
-  //     Module will get different discriminators than it would have if it were<br>
-  //     generated in its own context.<br>
-  //  2. If this function is called after round-tripping to bitcode instead of<br>
-  //     before, it will give a different (and potentially incorrect!) return.<br>
-  //<br>
-  // The discriminator should instead be calculated from local information<br>
-  // where it's actually needed.  This logic should be moved to<br>
-  // AddDiscriminators::runOnFunction(), where it doesn't pollute the<br>
-  // LLVMContext.<br>
-  std::pair<const char *, unsigned> Key(getFilename().data(), getLine());<br>
-  return ++getContext().pImpl->DiscriminatorTable[Key];<br>
-}<br>
-<br>
 unsigned DINode::getFlag(StringRef Flag) {<br>
   return StringSwitch<unsigned>(Flag)<br>
 #define HANDLE_DI_FLAG(ID, NAME) .Case("DIFlag" #NAME, Flag##NAME)<br>
Index: include/llvm/IR/DebugInfoMetadata.h<br>
===================================================================<br>
--- include/llvm/IR/DebugInfoMetadata.h<br>
+++ include/llvm/IR/DebugInfoMetadata.h<br>
@@ -1192,15 +1192,6 @@<br>
   /// instructions that are on different basic blocks.<br>
   inline unsigned getDiscriminator() const;<br>
<br>
-  /// \brief Compute new discriminator in the given context.<br>
-  ///<br>
-  /// This modifies the \a LLVMContext that \c this is in to increment the next<br>
-  /// discriminator for \c this's line/filename combination.<br>
-  ///<br>
-  /// FIXME: Delete this.  See comments in implementation and at the only call<br>
-  /// site in \a AddDiscriminators::runOnFunction().<br>
-  unsigned computeNewDiscriminator() const;<br>
-<br>
   Metadata *getRawScope() const { return getOperand(0); }<br>
   Metadata *getRawInlinedAt() const {<br>
     if (getNumOperands() == 2)<br>
<br>
<br>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>