[Mem2Reg] Create merged debug locations for inserted phis

[vsk]

Summary

Track the debug locations of the incoming values to newly-created phis,
and apply merged debug locations to the phis.

A merged location will be on line 0, but will have the correct scope
set. This improves crash reporting when an inlined instruction with a
merged location triggers a machine exception. A debugger will be able to
narrow down the crash to the correct inlined scope, instead of simply
pointing to the outer scope of the caller.

Taken together with a change allows generating merged line-0 locations
for instructions which aren't calls, this results in a 0.5% increase in
the uncompressed size of the .debug_line section of a stage2+Release
build of llc (-O3 -g).

Depends on https://reviews.llvm.org/D45396.

rdar://33858697

Edit: The 0.5% size increase of the .debug_line section was for llc. I typed
clang mistakenly. The number for clang is 0.6%.

[vsk]

• Removed an unused #include.

[aprantl]

lib/Transforms/Utils/PromoteMemoryToRegister.cpp

552 ↗

(On Diff #141464)

I don't think we can use this condition. LLVM supports (((debug info) being inlined into a nodebug function) being inlined into a function with debug info). You may need to just unconditionally turn this on.

[vsk]

Merge debug locs unconditionally, per Adrian's comment.

[vsk]

Ping.

[aprantl]

lib/Transforms/Utils/PromoteMemoryToRegister.cpp

167 ↗

(On Diff #141911)

///

169 ↗

(On Diff #141911)

might as well make this a struct?

[vsk]

Thanks!

lib/Transforms/Utils/PromoteMemoryToRegister.cpp

169 ↗

(On Diff #141911)

I'll do this in a prep commit.

llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp

void AnalyzeAlloca(AllocaInst *AI) {
 
     DbgDeclares = FindDbgAddrUses(AI);
   }
 };
 
 /// Data package used by RenamePass().
 struct RenamePassData {
   using ValVector = std::vector<Value *>;
+  using LocationVector = std::vector<DebugLoc>;
 
-  RenamePassData(BasicBlock *B, BasicBlock *P, ValVector V)
-      : BB(B), Pred(P), Values(std::move(V)) {}
+  RenamePassData(BasicBlock *B, BasicBlock *P, ValVector V, LocationVector L)
+      : BB(B), Pred(P), Values(std::move(V)), Locations(std::move(L)) {}
 
   BasicBlock *BB;
   BasicBlock *Pred;
   ValVector Values;
+  LocationVector Locations;
 };
 
 /// \brief This assigns and keeps a per-bb relative ordering of load/store
 /// instructions in the block that directly load or store an alloca.
 ///
 /// This functionality is important because it avoids scanning large basic
 /// blocks multiple times when promoting many allocas in the same block.
 class LargeBlockInfo {
unsigned getNumPreds(const BasicBlock *BB) {
     return NP - 1;
   }
 
   void ComputeLiveInBlocks(AllocaInst *AI, AllocaInfo &Info,
                            const SmallPtrSetImpl<BasicBlock *> &DefBlocks,
                            SmallPtrSetImpl<BasicBlock *> &LiveInBlocks);
   void RenamePass(BasicBlock *BB, BasicBlock *Pred,
                   RenamePassData::ValVector &IncVals,
+                  RenamePassData::LocationVector &IncLocs,
                   std::vector<RenamePassData> &Worklist);
   bool QueuePhiNode(BasicBlock *BB, unsigned AllocaIdx, unsigned &Version);
 };
 
 } // end anonymous namespace
 
 /// Given a LoadInst LI this adds assume(LI != null) after it.
 static void addAssumeNonNull(AssumptionCache *AC, LoadInst *LI) {
void PromoteMem2Reg::run() {
 
   // Set the incoming values for the basic block to be null values for all of
   // the alloca's.  We do this in case there is a load of a value that has not
   // been stored yet.  In this case, it will get this null value.
   RenamePassData::ValVector Values(Allocas.size());
   for (unsigned i = 0, e = Allocas.size(); i != e; ++i)
     Values[i] = UndefValue::get(Allocas[i]->getAllocatedType());
 
+  // When handling debug info, treat all incoming values as if they have unknown
+  // locations until proven otherwise.
+  RenamePassData::LocationVector Locations(Allocas.size());
+
   // Walks all basic blocks in the function performing the SSA rename algorithm
   // and inserting the phi nodes we marked as necessary
   std::vector<RenamePassData> RenamePassWorkList;
-  RenamePassWorkList.emplace_back(&F.front(), nullptr, std::move(Values));
+  RenamePassWorkList.emplace_back(&F.front(), nullptr, std::move(Values),
+                                  std::move(Locations));
   do {
     RenamePassData RPD = std::move(RenamePassWorkList.back());
     RenamePassWorkList.pop_back();
     // RenamePass may add new worklist entries.
-    RenamePass(RPD.BB, RPD.Pred, RPD.Values, RenamePassWorkList);
+    RenamePass(RPD.BB, RPD.Pred, RPD.Values, RPD.Locations, RenamePassWorkList);
   } while (!RenamePassWorkList.empty());
 
   // The renamer uses the Visited set to avoid infinite loops.  Clear it now.
   Visited.clear();
 
   // Remove the allocas themselves from the function.
   for (Instruction *A : Allocas) {
     // If there are any uses of the alloca instructions left, they must be in
bool PromoteMem2Reg::QueuePhiNode(BasicBlock *BB, unsigned AllocaNo,
   PN = PHINode::Create(Allocas[AllocaNo]->getAllocatedType(), getNumPreds(BB),
                        Allocas[AllocaNo]->getName() + "." + Twine(Version++),
                        &BB->front());
   ++NumPHIInsert;
   PhiToAllocaMap[PN] = AllocaNo;
   return true;
 }
 
+/// Update the debug location of a phi. \p ApplyMergedLoc indicates whether to
+/// create a merged location incorporating \p DL, or to set \p DL directly.
+static void updateForIncomingValueLocation(PHINode *PN, DebugLoc DL,
+                                           bool ApplyMergedLoc) {
+  if (ApplyMergedLoc)
+    PN->applyMergedLocation(PN->getDebugLoc(), DL);
+  else
+    PN->setDebugLoc(DL);
+}
+
 /// \brief Recursively traverse the CFG of the function, renaming loads and
 /// stores to the allocas which we are promoting.
 ///
 /// IncomingVals indicates what value each Alloca contains on exit from the
 /// predecessor block Pred.
 void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
                                 RenamePassData::ValVector &IncomingVals,
+                                RenamePassData::LocationVector &IncomingLocs,
                                 std::vector<RenamePassData> &Worklist) {
 NextIteration:
   // If we are inserting any phi nodes into this BB, they will already be in the
   // block.
   if (PHINode *APN = dyn_cast<PHINode>(BB->begin())) {
     // If we have PHI nodes to update, compute the number of edges from Pred to
     // BB.
     if (PhiToAllocaMap.count(APN)) {
       // We want to be able to distinguish between PHI nodes being inserted by
       // this invocation of mem2reg from those phi nodes that already existed in
       // the IR before mem2reg was run.  We determine that APN is being inserted
       // because it is missing incoming edges.  All other PHI nodes being
       // inserted by this pass of mem2reg will have the same number of incoming
       // operands so far.  Remember this count.
       unsigned NewPHINumOperands = APN->getNumOperands();
 
       unsigned NumEdges = std::count(succ_begin(Pred), succ_end(Pred), BB);
       assert(NumEdges && "Must be at least one edge from Pred to BB!");
 
       // Add entries for all the phis.
       BasicBlock::iterator PNI = BB->begin();
       do {
         unsigned AllocaNo = PhiToAllocaMap[APN];
 
+        // Update the location of the phi node.
+        updateForIncomingValueLocation(APN, IncomingLocs[AllocaNo],
+                                       APN->getNumIncomingValues() > 0);
+
         // Add N incoming values to the PHI node.
         for (unsigned i = 0; i != NumEdges; ++i)
           APN->addIncoming(IncomingVals[AllocaNo], Pred);
 
         // The currently active variable for this block is now the PHI.
         IncomingVals[AllocaNo] = APN;
         for (DbgInfoIntrinsic *DII : AllocaDbgDeclares[AllocaNo])
           ConvertDebugDeclareToDebugValue(DII, APN, DIB);
if (LoadInst *LI = dyn_cast<LoadInst>(I)) {
       if (!Dest)
         continue;
 
       DenseMap<AllocaInst *, unsigned>::iterator ai = AllocaLookup.find(Dest);
       if (ai == AllocaLookup.end())
         continue;
 
       // what value were we writing?
-      IncomingVals[ai->second] = SI->getOperand(0);
+      unsigned AllocaNo = ai->second;
+      IncomingVals[AllocaNo] = SI->getOperand(0);
+
       // Record debuginfo for the store before removing it.
+      IncomingLocs[AllocaNo] = SI->getDebugLoc();
       for (DbgInfoIntrinsic *DII : AllocaDbgDeclares[ai->second])
         ConvertDebugDeclareToDebugValue(DII, SI, DIB);
       BB->getInstList().erase(SI);
     }
   }
 
   // 'Recurse' to our successors.
   succ_iterator I = succ_begin(BB), E = succ_end(BB);
   if (I == E)
     return;
 
   // Keep track of the successors so we don't visit the same successor twice
   SmallPtrSet<BasicBlock *, 8> VisitedSuccs;
 
   // Handle the first successor without using the worklist.
   VisitedSuccs.insert(*I);
   Pred = BB;
   BB = *I;
   ++I;
 
   for (; I != E; ++I)
     if (VisitedSuccs.insert(*I).second)
-      Worklist.emplace_back(*I, Pred, IncomingVals);
+      Worklist.emplace_back(*I, Pred, IncomingVals, IncomingLocs);
 
   goto NextIteration;
 }
 
 void llvm::PromoteMemToReg(ArrayRef<AllocaInst *> Allocas, DominatorTree &DT,
                            AssumptionCache *AC) {
   // If there is nothing to do, bail out...
   if (Allocas.empty())
     return;
 
   PromoteMem2Reg(Allocas, DT, AC).run();
 }

llvm/trunk/test/Transforms/Mem2Reg/dbg-inline-scope-for-phi.ll

+; RUN: opt -S < %s -mem2reg -verify | FileCheck %s
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.13.0"
+
+; Original source (with some whitespace removed):
+;
+;   extern int *getp();
+;   extern int cond();
+;   int get1() { return *getp(); }
+;   int get2(int *p) { return *p; }
+;   int bug(int *p) {
+;     if (cond()) return get1();
+;     else return get2(p);
+;   }
+
+define i32 @get1() !dbg !8 {
+  %1 = call i32* (...) @getp(), !dbg !12
+  %2 = load i32, i32* %1, align 4, !dbg !13
+  ret i32 %2, !dbg !14
+}
+
+declare i32* @getp(...)
+
+define i32 @get2(i32*) !dbg !15 {
+  %2 = alloca i32*, align 8
+  store i32* %0, i32** %2, align 8
+  call void @llvm.dbg.declare(metadata i32** %2, metadata !19, metadata !DIExpression()), !dbg !20
+  %3 = load i32*, i32** %2, align 8, !dbg !21
+  %4 = load i32, i32* %3, align 4, !dbg !22
+  ret i32 %4, !dbg !23
+}
+
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+; CHECK-LABEL: define i32 @bug
+define i32 @bug(i32*) !dbg !24 {
+  %2 = alloca i32, align 4
+  %3 = alloca i32*, align 8
+  store i32* %0, i32** %3, align 8
+  call void @llvm.dbg.declare(metadata i32** %3, metadata !25, metadata !DIExpression()), !dbg !26
+  %4 = call i32 (...) @cond(), !dbg !27
+  %5 = icmp ne i32 %4, 0, !dbg !27
+  br i1 %5, label %6, label %8, !dbg !29
+
+; <label>:6:                                      ; preds = %1
+  %7 = call i32 @get1(), !dbg !30
+  store i32 %7, i32* %2, align 4, !dbg !31
+  br label %11, !dbg !31
+
+; <label>:8:                                      ; preds = %1
+  %9 = load i32*, i32** %3, align 8, !dbg !32
+  %10 = call i32 @get2(i32* %9), !dbg !33
+  store i32 %10, i32* %2, align 4, !dbg !34
+  br label %11, !dbg !34
+
+; <label>:11:                                     ; preds = %8, %6
+  %12 = load i32, i32* %2, align 4, !dbg !35
+  ret i32 %12, !dbg !35
+
+  ; CHECK: [[phi:%.*]] = phi i32 [ {{.*}} ], [ {{.*}} ], !dbg [[mergedLoc:![0-9]+]]
+  ; CHECK-NEXT: ret i32 [[phi]], !dbg [[retLoc:![0-9]+]]
+}
+
+; CHECK: [[commonScope:![0-9]+]] = distinct !DILexicalBlock(scope: {{.*}}, file: !1, line: 15, column: 7)
+; CHECK: [[mergedLoc]] = !DILocation(line: 0, scope: [[commonScope]])
+; CHECK: [[retLoc]] = !DILocation(line: 23, column: 1
+
+declare i32 @cond(...)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "Apple LLVM version 9.1.0 (clang-902.2.37.2)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "bug.c", directory: "/bug")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = !{!"Apple LLVM version 9.1.0 (clang-902.2.37.2)"}
+!8 = distinct !DISubprogram(name: "get1", scope: !1, file: !1, line: 6, type: !9, isLocal: false, isDefinition: true, scopeLine: 6, isOptimized: false, unit: !0, variables: !2)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !DILocation(line: 7, column: 11, scope: !8)
+!13 = !DILocation(line: 7, column: 10, scope: !8)
+!14 = !DILocation(line: 7, column: 3, scope: !8)
+!15 = distinct !DISubprogram(name: "get2", scope: !1, file: !1, line: 10, type: !16, isLocal: false, isDefinition: true, scopeLine: 10, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
+!16 = !DISubroutineType(types: !17)
+!17 = !{!11, !18}
+!18 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !11, size: 64)
+!19 = !DILocalVariable(name: "p", arg: 1, scope: !15, file: !1, line: 10, type: !18)
+!20 = !DILocation(line: 10, column: 15, scope: !15)
+!21 = !DILocation(line: 11, column: 11, scope: !15)
+!22 = !DILocation(line: 11, column: 10, scope: !15)
+!23 = !DILocation(line: 11, column: 3, scope: !15)
+!24 = distinct !DISubprogram(name: "bug", scope: !1, file: !1, line: 14, type: !16, isLocal: false, isDefinition: true, scopeLine: 14, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
+!25 = !DILocalVariable(name: "p", arg: 1, scope: !24, file: !1, line: 14, type: !18)
+!26 = !DILocation(line: 14, column: 14, scope: !24)
+!27 = !DILocation(line: 15, column: 7, scope: !28)
+!28 = distinct !DILexicalBlock(scope: !24, file: !1, line: 15, column: 7)
+!29 = !DILocation(line: 15, column: 7, scope: !24)
+!30 = !DILocation(line: 16, column: 12, scope: !28)
+!31 = !DILocation(line: 16, column: 5, scope: !28)
+!32 = !DILocation(line: 18, column: 17, scope: !28)
+!33 = !DILocation(line: 18, column: 12, scope: !28)
+!34 = !DILocation(line: 18, column: 5, scope: !28)
+!35 = !DILocation(line: 23, column: 1, scope: !24)