DebugInfo: Require a DebugLoc in DIBuilder::insertDeclare()

Change `DIBuilder::insertDeclare()` and `insertDbgValueIntrinsic()` to
take an `MDLocation*`/`DebugLoc` parameter which it attaches to the
created intrinsic.  Assert at creation time that the `scope:` field's
subprogram matches the variable's.  There's a matching `clang` commit to
use the API.

The context for this is PR22778, which is removing the `inlinedAt:`
field from `MDLocalVariable`, instead deferring to the `!dbg` location
attached to the debug info intrinsic.  The best way to ensure we always
have a `!dbg` attachment is to require one at creation time.  I'll be
adding verifier checks next, but this API change is the best way to
shake out frontend bugs.

Note: I added an `llvm_unreachable()` in `bindings/go` and passed in
`nullptr` for the `DebugLoc`.  The `llgo` folks will eventually need to
pass a valid `DebugLoc` here.

llvm-svn: 235041

llvm/bindings/go/llvm/DIBuilderBindings.cpp

@@ -234,10 +234,14 @@ LLVMValueRef LLVMDIBuilderInsertDeclareAtEnd(LLVMDIBuilderRef Dref,
                                              LLVMMetadataRef VarInfo,
                                              LLVMMetadataRef Expr,
                                              LLVMBasicBlockRef Block) {
+  // Fail immediately here until the llgo folks update their bindings.  The
+  // called function is going to assert out anyway.
+  llvm_unreachable("DIBuilder API change requires a DebugLoc");
+
   DIBuilder *D = unwrap(Dref);
-  Instruction *Instr =
+  Instruction *Instr = D->insertDeclare(
-      D->insertDeclare(unwrap(Storage), unwrap<MDLocalVariable>(VarInfo),
+      unwrap(Storage), unwrap<MDLocalVariable>(VarInfo),
-                       unwrap<MDExpression>(Expr), unwrap(Block));
+      unwrap<MDExpression>(Expr), /* DebugLoc */ nullptr, unwrap(Block));
   return wrap(Instr);
 }
 
@@ -246,9 +250,13 @@ LLVMValueRef LLVMDIBuilderInsertValueAtEnd(LLVMDIBuilderRef Dref,
                                            LLVMMetadataRef VarInfo,
                                            LLVMMetadataRef Expr,
                                            LLVMBasicBlockRef Block) {
+  // Fail immediately here until the llgo folks update their bindings.  The
+  // called function is going to assert out anyway.
+  llvm_unreachable("DIBuilder API change requires a DebugLoc");
+
   DIBuilder *D = unwrap(Dref);
   Instruction *Instr = D->insertDbgValueIntrinsic(
       unwrap(Val), Offset, unwrap<MDLocalVariable>(VarInfo),
-      unwrap<MDExpression>(Expr), unwrap(Block));
+      unwrap<MDExpression>(Expr), /* DebugLoc */ nullptr, unwrap(Block));
   return wrap(Instr);
 }

llvm/include/llvm/IR/DIBuilder.h

@@ -656,26 +656,32 @@ namespace llvm {
     /// @param Storage     llvm::Value of the variable
     /// @param VarInfo     Variable's debug info descriptor.
     /// @param Expr         A complex location expression.
+    /// @param DL           Debug info location.
     /// @param InsertAtEnd Location for the new intrinsic.
     Instruction *insertDeclare(llvm::Value *Storage, DIVariable VarInfo,
-                               DIExpression Expr, BasicBlock *InsertAtEnd);
+                               DIExpression Expr, const MDLocation *DL,
+                               BasicBlock *InsertAtEnd);
 
     /// insertDeclare - Insert a new llvm.dbg.declare intrinsic call.
     /// @param Storage      llvm::Value of the variable
     /// @param VarInfo      Variable's debug info descriptor.
     /// @param Expr         A complex location expression.
+    /// @param DL           Debug info location.
     /// @param InsertBefore Location for the new intrinsic.
     Instruction *insertDeclare(llvm::Value *Storage, DIVariable VarInfo,
-                               DIExpression Expr, Instruction *InsertBefore);
+                               DIExpression Expr, const MDLocation *DL,
+                               Instruction *InsertBefore);
 
     /// insertDbgValueIntrinsic - Insert a new llvm.dbg.value intrinsic call.
     /// @param Val          llvm::Value of the variable
     /// @param Offset       Offset
     /// @param VarInfo      Variable's debug info descriptor.
     /// @param Expr         A complex location expression.
+    /// @param DL           Debug info location.
     /// @param InsertAtEnd Location for the new intrinsic.
     Instruction *insertDbgValueIntrinsic(llvm::Value *Val, uint64_t Offset,
                                          DIVariable VarInfo, DIExpression Expr,
+                                         const MDLocation *DL,
                                          BasicBlock *InsertAtEnd);
 
     /// insertDbgValueIntrinsic - Insert a new llvm.dbg.value intrinsic call.
@@ -683,9 +689,11 @@ namespace llvm {
     /// @param Offset       Offset
     /// @param VarInfo      Variable's debug info descriptor.
     /// @param Expr         A complex location expression.
+    /// @param DL           Debug info location.
     /// @param InsertBefore Location for the new intrinsic.
     Instruction *insertDbgValueIntrinsic(llvm::Value *Val, uint64_t Offset,
                                          DIVariable VarInfo, DIExpression Expr,
+                                         const MDLocation *DL,
                                          Instruction *InsertBefore);
 
     /// \brief Replace the vtable holder in the given composite type.

llvm/lib/IR/DIBuilder.cpp

@@ -761,10 +761,19 @@ static Value *getDbgIntrinsicValueImpl(LLVMContext &VMContext, Value *V) {
   return MetadataAsValue::get(VMContext, ValueAsMetadata::get(V));
 }
 
+static Instruction *withDebugLoc(Instruction *I, const MDLocation *DL) {
+  I->setDebugLoc(const_cast<MDLocation *>(DL));
+  return I;
+}
+
 Instruction *DIBuilder::insertDeclare(Value *Storage, DIVariable VarInfo,
-                                      DIExpression Expr,
+                                      DIExpression Expr, const MDLocation *DL,
                                       Instruction *InsertBefore) {
   assert(VarInfo && "empty or invalid DIVariable passed to dbg.declare");
+  assert(DL && "Expected debug loc");
+  assert(DL->getScope()->getSubprogram() ==
+             VarInfo->getScope()->getSubprogram() &&
+         "Expected matching subprograms");
   if (!DeclareFn)
     DeclareFn = Intrinsic::getDeclaration(&M, Intrinsic::dbg_declare);
 
@@ -773,13 +782,17 @@ Instruction *DIBuilder::insertDeclare(Value *Storage, DIVariable VarInfo,
   Value *Args[] = {getDbgIntrinsicValueImpl(VMContext, Storage),
                    MetadataAsValue::get(VMContext, VarInfo),
                    MetadataAsValue::get(VMContext, Expr)};
-  return CallInst::Create(DeclareFn, Args, "", InsertBefore);
+  return withDebugLoc(CallInst::Create(DeclareFn, Args, "", InsertBefore), DL);
 }
 
 Instruction *DIBuilder::insertDeclare(Value *Storage, DIVariable VarInfo,
-                                      DIExpression Expr,
+                                      DIExpression Expr, const MDLocation *DL,
                                       BasicBlock *InsertAtEnd) {
   assert(VarInfo && "empty or invalid DIVariable passed to dbg.declare");
+  assert(DL && "Expected debug loc");
+  assert(DL->getScope()->getSubprogram() ==
+             VarInfo->getScope()->getSubprogram() &&
+         "Expected matching subprograms");
   if (!DeclareFn)
     DeclareFn = Intrinsic::getDeclaration(&M, Intrinsic::dbg_declare);
 
@@ -792,17 +805,21 @@ Instruction *DIBuilder::insertDeclare(Value *Storage, DIVariable VarInfo,
   // If this block already has a terminator then insert this intrinsic
   // before the terminator.
   if (TerminatorInst *T = InsertAtEnd->getTerminator())
-    return CallInst::Create(DeclareFn, Args, "", T);
+    return withDebugLoc(CallInst::Create(DeclareFn, Args, "", T), DL);
-  else
+  return withDebugLoc(CallInst::Create(DeclareFn, Args, "", InsertAtEnd), DL);
-    return CallInst::Create(DeclareFn, Args, "", InsertAtEnd);
 }
 
 Instruction *DIBuilder::insertDbgValueIntrinsic(Value *V, uint64_t Offset,
                                                 DIVariable VarInfo,
                                                 DIExpression Expr,
+                                                const MDLocation *DL,
                                                 Instruction *InsertBefore) {
   assert(V && "no value passed to dbg.value");
   assert(VarInfo && "empty or invalid DIVariable passed to dbg.value");
+  assert(DL && "Expected debug loc");
+  assert(DL->getScope()->getSubprogram() ==
+             VarInfo->getScope()->getSubprogram() &&
+         "Expected matching subprograms");
   if (!ValueFn)
     ValueFn = Intrinsic::getDeclaration(&M, Intrinsic::dbg_value);
 
@@ -812,15 +829,20 @@ Instruction *DIBuilder::insertDbgValueIntrinsic(Value *V, uint64_t Offset,
                    ConstantInt::get(Type::getInt64Ty(VMContext), Offset),
                    MetadataAsValue::get(VMContext, VarInfo),
                    MetadataAsValue::get(VMContext, Expr)};
-  return CallInst::Create(ValueFn, Args, "", InsertBefore);
+  return withDebugLoc(CallInst::Create(ValueFn, Args, "", InsertBefore), DL);
 }
 
 Instruction *DIBuilder::insertDbgValueIntrinsic(Value *V, uint64_t Offset,
                                                 DIVariable VarInfo,
                                                 DIExpression Expr,
+                                                const MDLocation *DL,
                                                 BasicBlock *InsertAtEnd) {
   assert(V && "no value passed to dbg.value");
   assert(VarInfo && "empty or invalid DIVariable passed to dbg.value");
+  assert(DL && "Expected debug loc");
+  assert(DL->getScope()->getSubprogram() ==
+             VarInfo->getScope()->getSubprogram() &&
+         "Expected matching subprograms");
   if (!ValueFn)
     ValueFn = Intrinsic::getDeclaration(&M, Intrinsic::dbg_value);
 
@@ -830,7 +852,8 @@ Instruction *DIBuilder::insertDbgValueIntrinsic(Value *V, uint64_t Offset,
                    ConstantInt::get(Type::getInt64Ty(VMContext), Offset),
                    MetadataAsValue::get(VMContext, VarInfo),
                    MetadataAsValue::get(VMContext, Expr)};
-  return CallInst::Create(ValueFn, Args, "", InsertAtEnd);
+
+  return withDebugLoc(CallInst::Create(ValueFn, Args, "", InsertAtEnd), DL);
 }
 
 void DIBuilder::replaceVTableHolder(DICompositeType &T, DICompositeType VTableHolder) {

llvm/lib/Transforms/Scalar/SROA.cpp

@@ -1166,10 +1166,9 @@ public:
       } else {
         continue;
       }
-      Instruction *DbgVal =
+      DIB.insertDbgValueIntrinsic(Arg, 0, DIVariable(DVI->getVariable()),
-          DIB.insertDbgValueIntrinsic(Arg, 0, DIVariable(DVI->getVariable()),
+                                  DIExpression(DVI->getExpression()),
-                                      DIExpression(DVI->getExpression()), Inst);
+                                  DVI->getDebugLoc(), Inst);
-      DbgVal->setDebugLoc(DVI->getDebugLoc());
     }
   }
 };
@@ -4211,8 +4210,8 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
       if (DbgDeclareInst *OldDDI = FindAllocaDbgDeclare(Piece.Alloca))
         OldDDI->eraseFromParent();
 
-      auto *NewDDI = DIB.insertDeclare(Piece.Alloca, Var, PieceExpr, &AI);
+      DIB.insertDeclare(Piece.Alloca, Var, PieceExpr, DbgDecl->getDebugLoc(),
-      NewDDI->setDebugLoc(DbgDecl->getDebugLoc());
+                        &AI);
     }
   }
   return Changed;

llvm/lib/Transforms/Scalar/ScalarReplAggregates.cpp

@@ -1117,10 +1117,9 @@ public:
       } else {
         continue;
       }
-      Instruction *DbgVal = DIB->insertDbgValueIntrinsic(
+      DIB->insertDbgValueIntrinsic(Arg, 0, DIVariable(DVI->getVariable()),
-          Arg, 0, DIVariable(DVI->getVariable()),
+                                   DIExpression(DVI->getExpression()),
-          DIExpression(DVI->getExpression()), Inst);
+                                   DVI->getDebugLoc(), Inst);
-      DbgVal->setDebugLoc(DVI->getDebugLoc());
     }
   }
 };

llvm/lib/Transforms/Utils/Local.cpp

@@ -1015,11 +1015,11 @@ bool llvm::ConvertDebugDeclareToDebugValue(DbgDeclareInst *DDI,
   if (SExtInst *SExt = dyn_cast<SExtInst>(SI->getOperand(0)))
     ExtendedArg = dyn_cast<Argument>(SExt->getOperand(0));
   if (ExtendedArg)
-    DbgVal = Builder.insertDbgValueIntrinsic(ExtendedArg, 0, DIVar, DIExpr, SI);
+    DbgVal = Builder.insertDbgValueIntrinsic(ExtendedArg, 0, DIVar, DIExpr,
+                                             DDI->getDebugLoc(), SI);
   else
     DbgVal = Builder.insertDbgValueIntrinsic(SI->getOperand(0), 0, DIVar,
-                                             DIExpr, SI);
+                                             DIExpr, DDI->getDebugLoc(), SI);
-  DbgVal->setDebugLoc(DDI->getDebugLoc());
   return true;
 }
 
@@ -1035,9 +1035,8 @@ bool llvm::ConvertDebugDeclareToDebugValue(DbgDeclareInst *DDI,
   if (LdStHasDebugValue(DIVar, LI))
     return true;
 
-  Instruction *DbgVal =
+  Builder.insertDbgValueIntrinsic(LI->getOperand(0), 0, DIVar, DIExpr,
-      Builder.insertDbgValueIntrinsic(LI->getOperand(0), 0, DIVar, DIExpr, LI);
+                                  DDI->getDebugLoc(), LI);
-  DbgVal->setDebugLoc(DDI->getDebugLoc());
   return true;
 }
 
@@ -1079,10 +1078,9 @@ bool llvm::LowerDbgDeclare(Function &F) {
           // This is a call by-value or some other instruction that
           // takes a pointer to the variable. Insert a *value*
           // intrinsic that describes the alloca.
-          auto DbgVal = DIB.insertDbgValueIntrinsic(
+          DIB.insertDbgValueIntrinsic(AI, 0, DIVariable(DDI->getVariable()),
-              AI, 0, DIVariable(DDI->getVariable()),
+                                      DIExpression(DDI->getExpression()),
-              DIExpression(DDI->getExpression()), CI);
+                                      DDI->getDebugLoc(), CI);
-          DbgVal->setDebugLoc(DDI->getDebugLoc());
         }
       DDI->eraseFromParent();
     }
@@ -1128,8 +1126,7 @@ bool llvm::replaceDbgDeclareForAlloca(AllocaInst *AI, Value *NewAllocaAddress,
   // Insert llvm.dbg.declare in the same basic block as the original alloca,
   // and remove old llvm.dbg.declare.
   BasicBlock *BB = AI->getParent();
-  Builder.insertDeclare(NewAllocaAddress, DIVar, DIExpr, BB)
+  Builder.insertDeclare(NewAllocaAddress, DIVar, DIExpr, Loc, BB);
-    ->setDebugLoc(Loc);
   DDI->eraseFromParent();
   return true;
 }

llvm/unittests/Transforms/Utils/Cloning.cpp

@@ -255,8 +255,10 @@ protected:
     DIExpression E = DBuilder.createExpression();
     DIVariable Variable = DBuilder.createLocalVariable(
       dwarf::DW_TAG_auto_variable, Subprogram, "x", File, 5, IntType, true);
-    DBuilder.insertDeclare(Alloca, Variable, E, Store);
+    auto *DL = MDLocation::get(Subprogram->getContext(), 5, 0, Subprogram);
-    DBuilder.insertDbgValueIntrinsic(AllocaContent, 0, Variable, E, Terminator);
+    DBuilder.insertDeclare(Alloca, Variable, E, DL, Store);
+    DBuilder.insertDbgValueIntrinsic(AllocaContent, 0, Variable, E, DL,
+                                     Terminator);
     // Finalize the debug info
     DBuilder.finalize();