Introduce updateDiscriminator interface to DILocation to make it cleaner assigning discriminators.

[danielcdh]

Summary

This patch introduces updateDiscriminator to DILocation so that it can be directly called by AddDiscriminator. It also makes it easier to update the discriminator later.

[aprantl]

include/llvm/IR/DebugInfoMetadata.h

1274

"update" doesn't feel quite right. Would cloneWithDiscriminator() or something along those lines be any better?

1620

We usually use auto if the type is obvious from the context.
I think a while-loop may be less compact but more readable here? Or maybe just add a comment explaining what the loop does?

[danielcdh]

update name and comment

[aprantl]

This really simplifies the code a great deal.

include/llvm/IR/DebugInfoMetadata.h

1274

Should we leave the inlining decision to the compiler?

1620

has -> have (2x)

[danielcdh]

Thanks for the quick review. Will hold this until tomorrow to see if other debug info maintainers have any comments about this API change.

[echristo]

I don't :)

-eric

[danielcdh]

fix some typo

include/llvm/IR/DebugInfoMetadata.h

public:
   }
 
   /// \brief Get the DWARF discriminator.
   ///
   /// DWARF discriminators distinguish identical file locations between
   /// instructions that are on different basic blocks.
   inline unsigned getDiscriminator() const;
 
+  /// Returns a new DILocation with updated \p Discriminator.
+  DILocation *cloneWithDiscriminator(unsigned Discriminator) const;
+
   Metadata *getRawScope() const { return getOperand(0); }
   Metadata *getRawInlinedAt() const {
     if (getNumOperands() == 2)
       return getOperand(1);
     return nullptr;
   }
 
   static bool classof(const Metadata *MD) {

 };
 
 unsigned DILocation::getDiscriminator() const {
   if (auto *F = dyn_cast<DILexicalBlockFile>(getScope()))
     return F->getDiscriminator();
   return 0;
 }
 
+DILocation *DILocation::cloneWithDiscriminator(unsigned Discriminator) const {
+  DIScope *Scope = getScope();
+  // Skip all parent DILexicalBlockFile that already have a discriminator
+  // assigned. We do not want to have nested DILexicalBlockFiles that have
+  // mutliple discriminators because only the leaf DILexicalBlockFile's
+  // dominator will be used.
+  for (auto *LBF = dyn_cast<DILexicalBlockFile>(Scope);
+       LBF && LBF->getDiscriminator() != 0;
+       LBF = dyn_cast<DILexicalBlockFile>(Scope))
+    Scope = LBF->getScope();
+  DILexicalBlockFile *NewScope =
+      DILexicalBlockFile::get(getContext(), Scope, getFile(), Discriminator);
+  return DILocation::get(getContext(), getLine(), getColumn(), NewScope,
+                         getInlinedAt());
+}
+
 class DINamespace : public DIScope {
   friend class LLVMContextImpl;
   friend class MDNode;
 
   unsigned Line;
 
   DINamespace(LLVMContext &Context, StorageType Storage, unsigned Line,
               ArrayRef<Metadata *> Ops)

lib/Transforms/Utils/AddDiscriminators.cpp

 // http://wiki.dwarfstd.org/index.php?title=Path_Discriminators
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Utils/AddDiscriminators.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Constants.h"
-#include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
-#include "llvm/IR/Module.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Scalar.h"
 
 using namespace llvm;
 

 static bool addDiscriminators(Function &F) {
   // If the function has debug information, but the user has disabled
   // discriminators, do nothing.
   // Simlarly, if the function has no debug info, do nothing.
   if (NoDiscriminators || !F.getSubprogram())
     return false;
 
   bool Changed = false;
-  Module *M = F.getParent();
-  LLVMContext &Ctx = M->getContext();
-  DIBuilder Builder(*M, /*AllowUnresolved*/ false);
 
   typedef std::pair<StringRef, unsigned> Location;
-  typedef SmallDenseMap<DIScope *, DILexicalBlockFile *, 1> ScopeMap;
-  typedef DenseMap<const BasicBlock *, ScopeMap> BBScopeMap;
-  typedef DenseMap<Location, BBScopeMap> LocationBBMap;
+  typedef DenseSet<const BasicBlock *> BBSet;
+  typedef DenseMap<Location, BBSet> LocationBBMap;
   typedef DenseMap<Location, unsigned> LocationDiscriminatorMap;
   typedef DenseSet<Location> LocationSet;
 
   LocationBBMap LBM;
   LocationDiscriminatorMap LDM;
 
   // Traverse all instructions in the function. If the source line location
   // of the instruction appears in other basic block, assign a new
   // discriminator for this instruction.
   for (BasicBlock &B : F) {
     for (auto &I : B.getInstList()) {
       if (isa<IntrinsicInst>(&I))
         continue;
       const DILocation *DIL = I.getDebugLoc();
       if (!DIL)
         continue;
-      DIScope *Scope = DIL->getScope();
       Location L = std::make_pair(DIL->getFilename(), DIL->getLine());
       auto &BBMap = LBM[L];
-      auto R = BBMap.insert({&B, ScopeMap()});
+      auto R = BBMap.insert(&B);
       if (BBMap.size() == 1)
         continue;
-      bool InsertSuccess = R.second;
-      ScopeMap &Scopes = R.first->second; 
       // If we could insert more than one block with the same line+file, a
       // discriminator is needed to distinguish both instructions.
-      auto R1 = Scopes.insert({Scope, nullptr});
-      DILexicalBlockFile *&NewScope = R1.first->second;
-      if (!NewScope) {
-        unsigned Discriminator = InsertSuccess ? ++LDM[L] : LDM[L];
-        auto *File = Builder.createFile(DIL->getFilename(),
-                                        Scope->getDirectory());
-        NewScope = Builder.createLexicalBlockFile(Scope, File, Discriminator);
-      }
-      I.setDebugLoc(DILocation::get(Ctx, DIL->getLine(), DIL->getColumn(),
-                                    NewScope, DIL->getInlinedAt()));
+      unsigned Discriminator = R.second ? ++LDM[L] : LDM[L];
+      I.setDebugLoc(DIL->cloneWithDiscriminator(Discriminator));
       DEBUG(dbgs() << DIL->getFilename() << ":" << DIL->getLine() << ":"
                    << DIL->getColumn() << ":"
-                   << dyn_cast<DILexicalBlockFile>(NewScope)->getDiscriminator()
+                   << Discriminator << " "
                    << I << "\n");
       Changed = true;
     }
   }
 
   // Traverse all instructions and assign new discriminators to call
   // instructions with the same lineno that are in the same basic block.
   // Sample base profile needs to distinguish different function calls within
   // a same source line for correct profile annotation.
   for (BasicBlock &B : F) {
     LocationSet CallLocations;
     for (auto &I : B.getInstList()) {
       CallInst *Current = dyn_cast<CallInst>(&I);
       if (!Current || isa<IntrinsicInst>(&I))
         continue;
 
       DILocation *CurrentDIL = Current->getDebugLoc();
       if (!CurrentDIL)
         continue;
       Location L =
           std::make_pair(CurrentDIL->getFilename(), CurrentDIL->getLine());
       if (!CallLocations.insert(L).second) {
-        auto *Scope = CurrentDIL->getScope();
-        auto *File = Builder.createFile(CurrentDIL->getFilename(),
-                                        Scope->getDirectory());
-        auto *NewScope = Builder.createLexicalBlockFile(Scope, File, ++LDM[L]);
-        Current->setDebugLoc(DILocation::get(Ctx, CurrentDIL->getLine(),
-                                             CurrentDIL->getColumn(), NewScope,
-                                             CurrentDIL->getInlinedAt()));
+        Current->setDebugLoc(CurrentDIL->updateDiscriminator(++LDM[L]));
         Changed = true;
       }
     }
   }
   return Changed;
 }
 
 bool AddDiscriminatorsLegacyPass::runOnFunction(Function &F) {

test/Transforms/AddDiscriminators/inlined.ll

 ; CHECK: ![[LB2]] = distinct !DILexicalBlock(scope: ![[LB3:[0-9]+]],
 ; CHECK-SAME:                                line: 1, column: 9)
 !16 = distinct !DILexicalBlock(scope: !17, file: !1, line: 1, column: 9)
 ; CHECK: ![[LB3]] = distinct !DILexicalBlock(scope: ![[F]],
 ; CHECK-SAME:                                line: 1, column: 9)
 !17 = distinct !DILexicalBlock(scope: !7, file: !1, line: 1, column: 9)
 !18 = !DILocation(line: 1, column: 9, scope: !7)
 !19 = !DILocation(line: 1, column: 27, scope: !12, inlinedAt: !13)
-; CHECK: ![[BR]] =  !DILocation(line: 1, column: 9, scope: ![[BF2:[0-9]+]])
-; CHECK: ![[BF2]] = !DILexicalBlockFile(scope: ![[BF]],
-; CHECK-SAME:                           discriminator: 1)
+; CHECK: ![[BR]] =  !DILocation(line: 1, column: 9, scope: !14)
 !20 = !DILocation(line: 1, column: 9, scope: !14)
 !21 = distinct !{!21, !18}
 !22 = !DILocation(line: 1, column: 56, scope: !12)
 !23 = !DILocation(line: 1, column: 27, scope: !12)
 !24 = !DILocation(line: 1, column: 84, scope: !12)