Update getMergedLocation to check the instruction type and merge properly.

Summary: If the merged instruction is call instruction, we need to set the scope to the closes common scope between 2 locations, otherwise it will cause trouble when the call is getting inlined.

Reviewers: dblaikie, aprantl

Reviewed By: dblaikie, aprantl

Subscribers: llvm-commits, sanjoy

Differential Revision: https://reviews.llvm.org/D37877

llvm-svn: 314694

llvm/include/llvm/IR/DebugInfoMetadata.h

@@ -1417,11 +1417,15 @@ public:
   /// could create a location with a new discriminator. If they are from
   /// different files/lines the location is ambiguous and can't be
   /// represented in a single line entry.  In this case, no location
-  /// should be set.
+  /// should be set, unless the merged instruction is a call, which we will
+  /// set the merged debug location as line 0 of the nearest common scope
+  /// where 2 locations are inlined from. This only applies to Instruction,
+  /// For MachineInstruction, as it is post-inline, we will treat the call
+  /// instruction the same way as other instructions.
   ///
-  /// Currently the function does not create a new location. If the locations
-  /// are the same, or cannot be discriminated, the first location is returned.
-  /// Otherwise an empty location will be used.
+  /// This should only be used by MachineInstruction because call can be
+  /// treated the same as other instructions. Otherwise, use
+  /// \p applyMergedLocation instead.
   static const DILocation *getMergedLocation(const DILocation *LocA,
                                              const DILocation *LocB) {
     if (LocA && LocB && (LocA == LocB || !LocA->canDiscriminate(*LocB)))

llvm/include/llvm/IR/Instruction.h

@@ -377,6 +377,21 @@ public:
   /// V and this instruction.
   void andIRFlags(const Value *V);
 
+  /// Merge 2 debug locations and apply it to the Instruction. If the
+  /// instruction is a CallIns, we need to traverse the inline chain to find
+  /// the common scope. This is not efficient for N-way merging as each time
+  /// you merge 2 iterations, you need to rebuild the hashmap to find the
+  /// common scope. However, we still choose this API because:
+  ///  1) Simplicity: it takes 2 locations instead of a list of locations.
+  ///  2) In worst case, it increases the complexity from O(N*I) to
+  ///     O(2*N*I), where N is # of Instructions to merge, and I is the
+  ///     maximum level of inline stack. So it is still linear.
+  ///  3) Merging of call instructions should be extremely rare in real
+  ///     applications, thus the N-way merging should be in code path.
+  /// The DebugLoc attached to this instruction will be overwritten by the
+  /// merged DebugLoc.
+  void applyMergedLocation(const DILocation *LocA, const DILocation *LocB);
+
 private:
   /// Return true if we have an entry in the on-the-side metadata hash.
   bool hasMetadataHashEntry() const {

llvm/lib/IR/DebugInfo.cpp

@@ -669,3 +669,26 @@ unsigned llvm::getDebugMetadataVersionFromModule(const Module &M) {
     return Val->getZExtValue();
   return 0;
 }
+
+void Instruction::applyMergedLocation(const DILocation *LocA,
+                                      const DILocation *LocB) {
+  if (LocA && LocB && (LocA == LocB || !LocA->canDiscriminate(*LocB))) {
+    setDebugLoc(LocA);
+    return;
+  }
+  if (!LocA || !LocB || !isa<CallInst>(this)) {
+    setDebugLoc(nullptr);
+    return;
+  }
+  SmallPtrSet<DILocation *, 5> InlinedLocationsA;
+  for (DILocation *L = LocA->getInlinedAt(); L; L = L->getInlinedAt())
+    InlinedLocationsA.insert(L);
+  const DILocation *Result = LocB;
+  for (DILocation *L = LocB->getInlinedAt(); L; L = L->getInlinedAt()) {
+    Result = L;
+    if (InlinedLocationsA.count(L))
+      break;
+  }
+  setDebugLoc(DILocation::get(
+      Result->getContext(), 0, 0, Result->getScope(), Result->getInlinedAt()));
+}

llvm/lib/Transforms/IPO/FunctionImport.cpp

@@ -463,6 +463,10 @@ void llvm::computeDeadSymbols(
   // Make value live and add it to the worklist if it was not live before.
   // FIXME: we should only make the prevailing copy live here
   auto visit = [&](ValueInfo VI) {
+    for (auto &S : VI.getSummaryList())
+      S->setLive(true);
+    ++LiveSymbols;
+    Worklist.push_back(VI);
     // FIXME: If we knew which edges were created for indirect call profiles,
     // we could skip them here. Any that are live should be reached via
     // other edges, e.g. reference edges. Otherwise, using a profile collected

llvm/lib/Transforms/InstCombine/InstCombineInternal.h

@@ -671,9 +671,9 @@ private:
   Instruction *FoldPHIArgLoadIntoPHI(PHINode &PN);
   Instruction *FoldPHIArgZextsIntoPHI(PHINode &PN);
 
-  /// Helper function for FoldPHIArgXIntoPHI() to get debug location for the
+  /// Helper function for FoldPHIArgXIntoPHI() to set debug location for the
   /// folded operation.
-  DebugLoc PHIArgMergedDebugLoc(PHINode &PN);
+  void PHIArgMergedDebugLoc(Instruction *Inst, PHINode &PN);
 
   Instruction *foldGEPICmp(GEPOperator *GEPLHS, Value *RHS,
                            ICmpInst::Predicate Cond, Instruction &I);

llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

@@ -1544,8 +1544,7 @@ bool InstCombiner::SimplifyStoreAtEndOfBlock(StoreInst &SI) {
                                    SI.getSyncScopeID());
   InsertNewInstBefore(NewSI, *BBI);
   // The debug locations of the original instructions might differ; merge them.
-  NewSI->setDebugLoc(DILocation::getMergedLocation(SI.getDebugLoc(),
-                                                   OtherStore->getDebugLoc()));
+  NewSI->applyMergedLocation(SI.getDebugLoc(), OtherStore->getDebugLoc());
 
   // If the two stores had AA tags, merge them.
   AAMDNodes AATags;

llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp

@@ -27,16 +27,17 @@ using namespace llvm::PatternMatch;
 /// The PHI arguments will be folded into a single operation with a PHI node
 /// as input. The debug location of the single operation will be the merged
 /// locations of the original PHI node arguments.
-DebugLoc InstCombiner::PHIArgMergedDebugLoc(PHINode &PN) {
+void InstCombiner::PHIArgMergedDebugLoc(Instruction *Inst, PHINode &PN) {
   auto *FirstInst = cast<Instruction>(PN.getIncomingValue(0));
-  const DILocation *Loc = FirstInst->getDebugLoc();
+  Inst->setDebugLoc(FirstInst->getDebugLoc());
+  // We do not expect a CallInst here, otherwise, N-way merging of DebugLoc
+  // will be inefficient.
+  assert(!isa<CallInst>(Inst));
 
   for (unsigned i = 1; i != PN.getNumIncomingValues(); ++i) {
     auto *I = cast<Instruction>(PN.getIncomingValue(i));
-    Loc = DILocation::getMergedLocation(Loc, I->getDebugLoc());
+    Inst->applyMergedLocation(Inst->getDebugLoc(), I->getDebugLoc());
   }
-
-  return Loc;
 }
 
 /// If we have something like phi [add (a,b), add(a,c)] and if a/b/c and the
@@ -117,7 +118,7 @@ Instruction *InstCombiner::FoldPHIArgBinOpIntoPHI(PHINode &PN) {
   if (CmpInst *CIOp = dyn_cast<CmpInst>(FirstInst)) {
     CmpInst *NewCI = CmpInst::Create(CIOp->getOpcode(), CIOp->getPredicate(),
                                      LHSVal, RHSVal);
-    NewCI->setDebugLoc(PHIArgMergedDebugLoc(PN));
+    PHIArgMergedDebugLoc(NewCI, PN);
     return NewCI;
   }
 
@@ -130,7 +131,7 @@ Instruction *InstCombiner::FoldPHIArgBinOpIntoPHI(PHINode &PN) {
   for (unsigned i = 1, e = PN.getNumIncomingValues(); i != e; ++i)
     NewBinOp->andIRFlags(PN.getIncomingValue(i));
 
-  NewBinOp->setDebugLoc(PHIArgMergedDebugLoc(PN));
+  PHIArgMergedDebugLoc(NewBinOp, PN);
   return NewBinOp;
 }
 
@@ -239,7 +240,7 @@ Instruction *InstCombiner::FoldPHIArgGEPIntoPHI(PHINode &PN) {
       GetElementPtrInst::Create(FirstInst->getSourceElementType(), Base,
                                 makeArrayRef(FixedOperands).slice(1));
   if (AllInBounds) NewGEP->setIsInBounds();
-  NewGEP->setDebugLoc(PHIArgMergedDebugLoc(PN));
+  PHIArgMergedDebugLoc(NewGEP, PN);
   return NewGEP;
 }
 
@@ -399,7 +400,7 @@ Instruction *InstCombiner::FoldPHIArgLoadIntoPHI(PHINode &PN) {
     for (Value *IncValue : PN.incoming_values())
       cast<LoadInst>(IncValue)->setVolatile(false);
 
-  NewLI->setDebugLoc(PHIArgMergedDebugLoc(PN));
+  PHIArgMergedDebugLoc(NewLI, PN);
   return NewLI;
 }
 
@@ -565,7 +566,7 @@ Instruction *InstCombiner::FoldPHIArgOpIntoPHI(PHINode &PN) {
   if (CastInst *FirstCI = dyn_cast<CastInst>(FirstInst)) {
     CastInst *NewCI = CastInst::Create(FirstCI->getOpcode(), PhiVal,
                                        PN.getType());
-    NewCI->setDebugLoc(PHIArgMergedDebugLoc(PN));
+    PHIArgMergedDebugLoc(NewCI, PN);
     return NewCI;
   }
 
@@ -576,14 +577,14 @@ Instruction *InstCombiner::FoldPHIArgOpIntoPHI(PHINode &PN) {
     for (unsigned i = 1, e = PN.getNumIncomingValues(); i != e; ++i)
       BinOp->andIRFlags(PN.getIncomingValue(i));
 
-    BinOp->setDebugLoc(PHIArgMergedDebugLoc(PN));
+    PHIArgMergedDebugLoc(BinOp, PN);
     return BinOp;
   }
 
   CmpInst *CIOp = cast<CmpInst>(FirstInst);
   CmpInst *NewCI = CmpInst::Create(CIOp->getOpcode(), CIOp->getPredicate(),
                                    PhiVal, ConstantOp);
-  NewCI->setDebugLoc(PHIArgMergedDebugLoc(PN));
+  PHIArgMergedDebugLoc(NewCI, PN);
   return NewCI;
 }
 

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

@@ -1277,9 +1277,7 @@ static bool HoistThenElseCodeToIf(BranchInst *BI,
 
     // I1 and I2 are being combined into a single instruction.  Its debug
     // location is the merged locations of the original instructions.
-    if (!isa<CallInst>(I1))
-      I1->setDebugLoc(
-          DILocation::getMergedLocation(I1->getDebugLoc(), I2->getDebugLoc()));
+    I1->applyMergedLocation(I1->getDebugLoc(), I2->getDebugLoc());
 
     I2->eraseFromParent();
     Changed = true;
@@ -1533,20 +1531,20 @@ static bool sinkLastInstruction(ArrayRef<BasicBlock*> Blocks) {
     I0->getOperandUse(O).set(NewOperands[O]);
   I0->moveBefore(&*BBEnd->getFirstInsertionPt());
 
-  // The debug location for the "common" instruction is the merged locations of
-  // all the commoned instructions.  We start with the original location of the
-  // "common" instruction and iteratively merge each location in the loop below.
-  const DILocation *Loc = I0->getDebugLoc();
-
   // Update metadata and IR flags, and merge debug locations.
   for (auto *I : Insts)
     if (I != I0) {
-      Loc = DILocation::getMergedLocation(Loc, I->getDebugLoc());
+      // The debug location for the "common" instruction is the merged locations
+      // of all the commoned instructions.  We start with the original location
+      // of the "common" instruction and iteratively merge each location in the
+      // loop below.
+      // This is an N-way merge, which will be inefficient if I0 is a CallInst.
+      // However, as N-way merge for CallInst is rare, so we use simplified API
+      // instead of using complex API for N-way merge.
+      I0->applyMergedLocation(I0->getDebugLoc(), I->getDebugLoc());
       combineMetadataForCSE(I0, I);
       I0->andIRFlags(I);
     }
-  if (!isa<CallInst>(I0))
-    I0->setDebugLoc(Loc);
 
   if (!isa<StoreInst>(I0)) {
     // canSinkLastInstruction checked that all instructions were used by
@@ -2030,9 +2028,8 @@ static bool SpeculativelyExecuteBB(BranchInst *BI, BasicBlock *ThenBB,
     Value *S = Builder.CreateSelect(
         BrCond, TrueV, FalseV, TrueV->getName() + "." + FalseV->getName(), BI);
     SpeculatedStore->setOperand(0, S);
-    SpeculatedStore->setDebugLoc(
-        DILocation::getMergedLocation(
-          BI->getDebugLoc(), SpeculatedStore->getDebugLoc()));
+    SpeculatedStore->applyMergedLocation(BI->getDebugLoc(),
+                                         SpeculatedStore->getDebugLoc());
   }
 
   // Metadata can be dependent on the condition we are hoisting above.

llvm/test/Transforms/SimplifyCFG/remove-debug.ll

@@ -2,14 +2,9 @@
 
 ; TODO: Track the acutal DebugLoc of the hoisted instruction when no-line
 ; DebugLoc is supported (https://reviews.llvm.org/D24180)
-; CHECK: line: 6
-; CHECK-NOT: line: 7
-; CHECK: line: 8
-; CHECK: line: 9
-; CHECK-NOT: line: 10
-; CHECK: line: 11
 
-; Checks if the debug info for hoisted "x = i" is removed
+; Checks if the debug info for hoisted "x = i" is removed and
+; the debug info for hoisted "bar()" is set as line 0
 ; int x;
 ; void bar();
 ; void baz();
@@ -20,6 +15,7 @@
 ;     bar();
 ;   } else {
 ;     x = i;
+;     bar();
 ;     baz();
 ;   }
 ; }
@@ -30,6 +26,10 @@ target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: uwtable
 define void @_Z3fooi(i32) #0 !dbg !6 {
+; CHECK: load i32, i32* %2, align 4, !tbaa
+; CHECK: store i32 %5, i32* @x, align 4, !tbaa
+; CHECK: call void @_Z3barv(), !dbg ![[BAR:[0-9]+]]
+; CHECK: call void @_Z3bazv(), !dbg ![[BAZ:[0-9]+]]
   %2 = alloca i32, align 4
   store i32 %0, i32* %2, align 4, !tbaa !8
   %3 = load i32, i32* %2, align 4, !dbg !12, !tbaa !8
@@ -45,7 +45,8 @@ define void @_Z3fooi(i32) #0 !dbg !6 {
 ; <label>:7:
   %8 = load i32, i32* %2, align 4, !dbg !18, !tbaa !8
   store i32 %8, i32* @x, align 4, !dbg !19, !tbaa !8
-  call void @_Z3bazv(), !dbg !20
+  call void @_Z3barv(), !dbg !20
+  call void @_Z3bazv(), !dbg !21
   br label %9
 
 ; <label>:9:
@@ -59,6 +60,8 @@ declare void @_Z3bazv() #1
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4}
 
+; CHECK: ![[BAR]] = !DILocation(line: 0
+; CHECK: ![[BAZ]] = !DILocation(line: 12, column: 5
 !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1)
 !1 = !DIFile(filename: "a", directory: "b/")
 !2 = !{}
@@ -80,4 +83,5 @@ declare void @_Z3bazv() #1
 !18 = !DILocation(line: 10, column: 9, scope: !6)
 !19 = !DILocation(line: 10, column: 7, scope: !6)
 !20 = !DILocation(line: 11, column: 5, scope: !6)
-!21 = !DILocation(line: 13, column: 1, scope: !6)
+!21 = !DILocation(line: 12, column: 5, scope: !6)
+!22 = !DILocation(line: 14, column: 1, scope: !6)