[IRBuilder] Set the insert point and debug location together

[vsk]

Summary

The APIs for setting the insertion point of an IRBuilder behave
inconsistently w.r.t updating the current debug location. The overloads
of SetInsertPoint() may or may not change the current debug location,
and may or may not select a correct location, depending on the selected
overload and the current state of the IRBuilder.

This is inconvenient and causes correctness problems. For more
background on this issue, see the thread:

[llvm-dev] [RFC] Setting the current debug loc when the insertion
point changes

This patch deprecates the following APIs:

SetInsertPoint(BasicBlock *)
SetInsertPoint(Instruction *)
SetInsertPoint(BasicBlock *, BasicBlock::iterator)

They are superseded by the following APIs:

setInsertPoint(BasicBlock *, const DebugLoc &)
setInsertPoint(Instruction *, const DebugLoc &)
setInsertPoint(BasicBlock *, BasicBlock::iterator, const DebugLoc &)
getUnknownDebugLocation(BasicBlock *, BasicBlock::iterator)
getUnknownDebugLocation(Instruction *)

The new APIs offer consistent handling of debug locations by requiring
clients to be more explicit. Uses of the old APIs should be audited and
phased out. New C APIs are available.

[vsk]

• Upload a diff with context.

[aprantl]

include/llvm/IR/IRBuilder.h

139

DebugLoc is already a value type, so we shouldn't pass it by reference.

148

out of curiosity: is there a doxygen command such as \deprecated?

169

either?

192

@{

196

@}

[aprantl]

Thanks, this is looking really good!

lib/IR/IRBuilder.cpp

25

Is falling back to getCurrentDebugLocation() a good idea? This looks like the kind of magic default behavior that this patch tries to get rid of.

[vsk]

• Address review feedback from Adrian.

include/llvm/IR/IRBuilder.h

139

Will fix.

148

Yes, I'll switch to that.

169

Will fix.

lib/IR/IRBuilder.cpp

25

No, it's not a good idea :), and it doesn't match what the comment says it does. Thanks for the catch. I added a check in the unit test which would fail if we used getCurrentDebugLocation() here.

[davide]

include/llvm/IR/IRBuilder.h

130

I'd really love to hear Eric (@echristo) input on this, but I have mixed feelings about deprecating an API and introducing a new API with the same name (and just a different overload).
It seems like people might still use this wrong, should we choose a different name?

lib/IR/Core.cpp

2447

The *2 variants are probably OK, and other projects seem to use it successfully.
http://man7.org/linux/man-pages/man2/rename.2.html
Not sure if we have a precedent, but it doesn't seem a bad path forward.

[vsk]

include/llvm/IR/IRBuilder.h

130

I'm open to changing the name here to make it more explicit, e.g setInsertPointAndDebugLoc.

lib/IR/Core.cpp

2447

I've seen this pattern floating around, e.g LLVMParseBitcodeInContext2, clang_createTranslationUnit2, etc. IMO '2' implies 'newer', which is good.

[echristo]

I've made a couple of comments on API naming and deprecating inline.

include/llvm-c/Core.h

2832

I really dislike the "2" moniker on APIs. Seems that if we want a new API that has a new behavior we should do that instead with new and descriptive names.

LLVMPositionBuilderWithLoc?

include/llvm/IR/IRBuilder.h

130

Why are we deprecating a C++ API at all? Just change it.

[davide]

lib/IR/Core.cpp

2447

In general these are slightly more counterintuitive to use.
I'd say people shouldn't be really concerned with the past when using an API. I have no strong feelings about it, but maybe it's better to choose a better name.

[vsk]

include/llvm-c/Core.h

2832

WithLoc sounds good to me, I'll update it.

include/llvm/IR/IRBuilder.h

130

Deprecating the existing API lets us switch to the new one incrementally, which would be less disruptive. If we don't mind some churn and would rather get this all done in one shot, I'm open to that (& can check in a script). @aprantl, @davide -- thoughts?

[aprantl]

include/llvm/IR/IRBuilder.h

130

I don't have a strong opinion. Whatever works best for you. If you are going to convert all uses anyway you might as do it in one go, or you can aim at smaller, incremental commits.

[vsk]

include/llvm/IR/IRBuilder.h

130

Well we need to provide an upgrade script anyway. And if we don't remove the old APIs now it's not clear when we will. I'm leaning towards updating everything in one shot.

[echristo]

All at once with an update tool is my preference. :)

Thanks!

-eric

[vsk]

After some consideration, I no longer think that we should provide an update tool or take the one-shot approach.

Here are the transforms a behavior-preserving update tool would need to make:

IRB.SetInsertPoint(TheBB) => IRB.setInsertPoint(TheBB, IRB.getCurrentDebugLocation())
IRB.SetInsertPoint(TheInst) => IRB.setInsertPoint(TheInst, TheInst->getDebugLoc())
IRB.SetInsertPoint(TheBB, IT) => IRB.setInsertPoint(TheBB, IT, IRB.getStickyDebugLocation(TheBB, IT)) // This one returns IRB.CurrentDbgLoc if TheBB->end() == IT, else IT->getDebugLoc().

A couple points about this:
o The update is fragile because arguments to SetInsertPoint are often potentially side-effecting
o We lose the ability to distinguish between audited and unaudited calls to SetInsertPoint
o We end up adding an API we don't want to promote ('getStickyDebugLocation')

@echristo and others -- in light of this could we go with the incremental route? We can release-note a deadline for upgrading existing calls to SetInsertPoint (say, llvm 6.0?), and I have time to audit these calls.

[vsk]

• Add a release note, and rename the C APIs so they use 'WithLoc' instead of '2'.

[vsk]

Friendly ping. I've outlined my thoughts on deprecating the old API versus updating the existing SetInsertPoint users above. Does anyone have feedback about that?

[whitequark]

include/llvm-c/Core.h

2832

Why should we deprecate these at all? It's legal to pass Loc = NULL for no location, right? Then why shouldn't we make e.g. LLVMPositionBuilderBefore(...) an alias to LLVMPositionBuilderBeforeWithLoc(..., NULL) and keep existing code working? Very few frontends in the wild bother with any debug information at all, I don't think it makes sense to break all of them.

[vsk]

include/llvm-c/Core.h

2832

We should deprecate these APIs because they aren't great for building frontends which do emit debug info, and because these APIs will (soon, hopefully) have no C++ counterparts.

It's legal to pass Loc = NULL, but that rewrite is only behavior-preserving for frontends which emit no debug info. To make the transition easier for these frontends, I can provide an upgrade script which rewrites PositionBuilder* to PositionBuilder*WithLoc(..., NULL). Does that sound OK to you?

[whitequark]

include/llvm-c/Core.h

2832

No, it does not sound OK because most LLVM frontends aren't written in C, they just use the C interface through FFI. Such a change would be onerous for these frontends for no good reason. I am not convinced that "not having an exact C++ counterpart" is a valid reason for deprecating a C API either.

I would be much more fine with plain out removing SetCurrentDebugLocation since that is used by fewer frontends.

[vsk]

include/llvm-c/Core.h

2832

I see only two alternatives to deprecating these C APIs: preserve their behavior or change it s.t they do not forward debug info. There's opposition to a silent behavior change on the mailing list and opposition to deprecating the APIs here.

That leaves the do nothing option: essentially, this patch sans the comments about deprecation. That's what I'll do. I'd be interested in any constructive alternatives you have.

[vsk]

• Do not mark any C APIs as deprecated.

[whitequark]

include/llvm-c/Core.h

2832

The patch looks better to me now. You could still deprecate SetCurrentDebugLocation so that once it is removed, it becomes a hard error for any FFI-based frontend to use it. I don't see any better option.

[vsk]

Ping, I'd love to find a path forward on this since it's a prerequisite for fixing bugs in LSR and SCEVExpander.

The main remaining issue is what to do with the old IRBuilder APIs for setting the insertion point. We discussed a couple options:

• I looked at converting all in-tree users to the new API in one shot. We have ~580 users, so this doesn't seem feasible to me without a deprecation period for the old APIs.
• I started working on an automatic update tool, but found that it's necessarily fragile (e.g when handling side-effecting arguments to API calls). In addition, it would require adding a confusing API to IRBuilder so that clients can mimic the 'sticky' behavior of one of SetInsertPoint's overloads.

Here are some ways I think we can move forward:

• Rename the new APIs to 'setInsertPointAndLoc' and keep the old APIs in perpetuity.
• Deprecate the old APIs for 1-2 releases and use the time to migrate in-tree users.

@echristo, @dblaikie, @aprantl and others: I'd like to hear your thoughts on this (especially if you have alternatives not listed here). Thanks!

[JDevlieghere]

In D39982#1010833, @vsk wrote:

Ping, I'd love to find a path forward on this since it's a prerequisite for fixing bugs in LSR and SCEVExpander.
...
Here are some ways I think we can move forward:

• Rename the new APIs to 'setInsertPointAndLoc' and keep the old APIs in perpetuity.
• Deprecate the old APIs for 1-2 releases and use the time to migrate in-tree users.

Assuming this is about the C++ APIs, I personally don't see any advantage of your first suggestion and I'd go with the current state of the diff. I also think it doesn't really matter all that much and certainly it's not worth blocking this review over. Either we fix all the in-tree uses by the next release in which case it doesn't matter, or we don't in which case nothing changes for downstream users. Sure, it might be confusing for the transition period, but that's true for both alternatives and at least it's better than what we currently have. Happy to hear other people's opinions if they disagree, but otherwise I'd love to see this getting checked in. As far as I'm concerned this LGTM.

docs/ReleaseNotes.rst

.. NOTE
    point (e.g. maybe you would like to give an example of the
    functionality, or simply have a lot to talk about), see the `NOTE` below
    for adding a new subsection.
 
 * The ``Redirects`` argument of ``llvm::sys::ExecuteAndWait`` and
   ``llvm::sys::ExecuteNoWait`` was changed to an ``ArrayRef`` of optional
   ``StringRef``'s to make it safer and more convenient to use.
 
+* The ``SetInsertPoint`` APIs in ``IRBuilderBase`` have been deprecated in
+  favor of APIs which explicitly set the current debug location along with the
+  insert point. These APIs will be removed in LLVM 7.0.
+
 * Note..
 
 .. NOTE
    If you would like to document a larger change, then you can add a
    subsection about it right here. You can copy the following boilerplate
    and un-indent it (the indentation causes it to be inside this comment).
 
    Special New Feature

include/llvm-c/Core.h

  * An instruction builder represents a point within a basic block and is
  * the exclusive means of building instructions using the C interface.
  *
  * @{
  */
 
 LLVMBuilderRef LLVMCreateBuilderInContext(LLVMContextRef C);
 LLVMBuilderRef LLVMCreateBuilder(void);
 void LLVMPositionBuilder(LLVMBuilderRef Builder, LLVMBasicBlockRef Block,
                          LLVMValueRef Instr);
 void LLVMPositionBuilderBefore(LLVMBuilderRef Builder, LLVMValueRef Instr);
 void LLVMPositionBuilderAtEnd(LLVMBuilderRef Builder, LLVMBasicBlockRef Block);
+void LLVMPositionBuilderWithLoc(LLVMBuilderRef Builder, LLVMBasicBlockRef Block,
+                                LLVMValueRef Instr, LLVMValueRef Loc);
+void LLVMPositionBuilderBeforeWithLoc(LLVMBuilderRef Builder,
+                                      LLVMValueRef Instr, LLVMValueRef Loc);
+void LLVMPositionBuilderAtEndWithLoc(LLVMBuilderRef Builder,
+                                     LLVMBasicBlockRef Block, LLVMValueRef Loc);
+LLVMValueRef LLVMGetUnknownDebugLocation(LLVMBuilderRef Builder,
+                                         LLVMBasicBlockRef Block,
+                                         LLVMValueRef Instr);
+LLVMValueRef LLVMGetUnknownDebugLocationBefore(LLVMBuilderRef Builder,
+                                               LLVMValueRef Instr);
 LLVMBasicBlockRef LLVMGetInsertBlock(LLVMBuilderRef Builder);
 void LLVMClearInsertionPosition(LLVMBuilderRef Builder);
 void LLVMInsertIntoBuilder(LLVMBuilderRef Builder, LLVMValueRef Instr);
 void LLVMInsertIntoBuilderWithName(LLVMBuilderRef Builder, LLVMValueRef Instr,
                                    const char *Name);
 void LLVMDisposeBuilder(LLVMBuilderRef Builder);
 
 /* Metadata */

include/llvm/IR/IRBuilder.h

public:
   }
 
   BasicBlock *GetInsertBlock() const { return BB; }
   BasicBlock::iterator GetInsertPoint() const { return InsertPt; }
   LLVMContext &getContext() const { return Context; }
 
   /// \brief This specifies that created instructions should be appended to the
   /// end of the specified block.
+  ///
+  /// \deprecated{Please use:
+  ///   setInsertPoint(BasicBlock *, DebugLoc)
+  /// }
   void SetInsertPoint(BasicBlock *TheBB) {
     BB = TheBB;
     InsertPt = BB->end();
   }
 
+  /// This specifies that created instructions should be appended to the end
+  /// of the specified block, and should have the given debug location.
+  void setInsertPoint(BasicBlock *TheBB, DebugLoc Loc) {
+    BB = TheBB;
+    InsertPt = BB->end();
+    SetCurrentDebugLocation(Loc);
+  }
+
   /// \brief This specifies that created instructions should be inserted before
   /// the specified instruction.
+  ///
+  /// \deprecated{Please use:
+  ///   setInsertPoint(Instruction *, DebugLoc)
+  /// }
   void SetInsertPoint(Instruction *I) {
     BB = I->getParent();
     InsertPt = I->getIterator();
     assert(InsertPt != BB->end() && "Can't read debug loc from end()");
     SetCurrentDebugLocation(I->getDebugLoc());
   }
 
+  /// This specifies that created instructions should be inserted before the
+  /// specified instruction, and should have the given debug location.
+  void setInsertPoint(Instruction *I, DebugLoc Loc) {
+    BB = I->getParent();
+    InsertPt = I->getIterator();
+    SetCurrentDebugLocation(Loc);
+  }
+
   /// \brief This specifies that created instructions should be inserted at the
   /// specified point.
+  ///
+  /// \deprecated{Please use:
+  ///   setInsertPoint(BasicBlock *, BasicBlock::iterator, DebugLoc)
+  /// }
   void SetInsertPoint(BasicBlock *TheBB, BasicBlock::iterator IP) {
     BB = TheBB;
     InsertPt = IP;
     if (IP != TheBB->end())
       SetCurrentDebugLocation(IP->getDebugLoc());
   }
 
+  /// This specifies that created instructions should be inserted at the
+  /// specified point, and should have the given debug location.
+  void setInsertPoint(BasicBlock *TheBB, BasicBlock::iterator IP,
+                      DebugLoc Loc) {
+    BB = TheBB;
+    InsertPt = IP;
+    SetCurrentDebugLocation(Loc);
+  }
+
+  /// Get the debug location which should be used for instructions without
+  /// a clear correspondence to a source location. This returns an empty debug
+  /// location when the specified insertion point is null or has no debug info
+  /// attached to it. Otherwise, this returns a <file>:0:0 location with the
+  /// DIScope set to the current inline scope.
+  /// @{
+  static DebugLoc getUnknownDebugLocation(Instruction *InsertPt = nullptr);
+  static DebugLoc getUnknownDebugLocation(BasicBlock *ForBB,
+                                          BasicBlock::iterator IP);
+  /// @}
+
   /// \brief Set location information used by debugging information.
   void SetCurrentDebugLocation(DebugLoc L) { CurDbgLocation = std::move(L); }
 
   /// \brief Get location information used by debugging information.
   const DebugLoc &getCurrentDebugLocation() const { return CurDbgLocation; }
 
   /// \brief If this builder has a current debug location, set it on the
   /// specified instruction.

lib/IR/Core.cpp

assert((isa<MDNode>(MD) || isa<ConstantAsMetadata>(MD)) &&
       "Expected a metadata node or a canonicalized constant");
 
   if (MDNode *N = dyn_cast<MDNode>(MD))
     return N;
 
   return MDNode::get(MAV->getContext(), MD);
 }
 
+/// Convert an LLVMValueRef to a DebugLoc.
+static DebugLoc getValueRefAsDebugLoc(LLVMValueRef L) {
+  return DebugLoc(L ? cast<MDNode>(unwrap<MetadataAsValue>(L)->getMetadata())
+                    : nullptr);
+}
+
 void LLVMSetMetadata(LLVMValueRef Inst, unsigned KindID, LLVMValueRef Val) {
   MDNode *N = Val ? extractMDNode(unwrap<MetadataAsValue>(Val)) : nullptr;
 
   unwrap<Instruction>(Inst)->setMetadata(KindID, N);
 }
 
 /*--.. Conversion functions ................................................--*/
 

 LLVMBuilderRef LLVMCreateBuilderInContext(LLVMContextRef C) {
   return wrap(new IRBuilder<>(*unwrap(C)));
 }
 
 LLVMBuilderRef LLVMCreateBuilder(void) {
   return LLVMCreateBuilderInContext(LLVMGetGlobalContext());
 }
 
+/// Set the insert point of the builder before \p IP. Apply a new debug
+/// location if one is available, and the current one if not.
+static void positionBuilderBefore(IRBuilderBase *IRB, BasicBlock *BB,
+                                  BasicBlock::iterator IP) {
+  if (IP == BB->end())
+    IRB->setInsertPoint(BB, IP, IRB->getCurrentDebugLocation());
+  else
+    IRB->setInsertPoint(BB, IP, IP->getDebugLoc());
+}
+
 void LLVMPositionBuilder(LLVMBuilderRef Builder, LLVMBasicBlockRef Block,
                          LLVMValueRef Instr) {
   BasicBlock *BB = unwrap(Block);
   auto I = Instr ? unwrap<Instruction>(Instr)->getIterator() : BB->end();
-  unwrap(Builder)->SetInsertPoint(BB, I);
+  positionBuilderBefore(unwrap(Builder), BB, I);
 }
 
 void LLVMPositionBuilderBefore(LLVMBuilderRef Builder, LLVMValueRef Instr) {
   Instruction *I = unwrap<Instruction>(Instr);
-  unwrap(Builder)->SetInsertPoint(I->getParent(), I->getIterator());
+  positionBuilderBefore(unwrap(Builder), I->getParent(), I->getIterator());
 }
 
 void LLVMPositionBuilderAtEnd(LLVMBuilderRef Builder, LLVMBasicBlockRef Block) {
   BasicBlock *BB = unwrap(Block);
-  unwrap(Builder)->SetInsertPoint(BB);
+  auto *IRB = unwrap(Builder);
+  IRB->setInsertPoint(BB, IRB->getCurrentDebugLocation());
+}
+
+void LLVMPositionBuilderWithLoc(LLVMBuilderRef Builder, LLVMBasicBlockRef Block,
+                          LLVMValueRef Instr, LLVMValueRef Loc) {
+  BasicBlock *BB = unwrap(Block);
+  auto I = Instr ? unwrap<Instruction>(Instr)->getIterator() : BB->end();
+  DebugLoc DL = getValueRefAsDebugLoc(Loc);
+  unwrap(Builder)->setInsertPoint(BB, I, DL);
+}
+
+void LLVMPositionBuilderBeforeWithLoc(LLVMBuilderRef Builder,
+                                      LLVMValueRef Instr, LLVMValueRef Loc) {
+  Instruction *I = unwrap<Instruction>(Instr);
+  DebugLoc DL = getValueRefAsDebugLoc(Loc);
+  unwrap(Builder)->setInsertPoint(I->getParent(), I->getIterator(), DL);
+}
+
+void LLVMPositionBuilderAtEndWithLoc(LLVMBuilderRef Builder,
+                                     LLVMBasicBlockRef Block,
+                                     LLVMValueRef Loc) {
+  BasicBlock *BB = unwrap(Block);
+  DebugLoc DL = getValueRefAsDebugLoc(Loc);
+  unwrap(Builder)->setInsertPoint(BB, DL);
+}
+
+LLVMValueRef LLVMGetUnknownDebugLocation(LLVMBuilderRef Builder,
+                                         LLVMBasicBlockRef Block,
+                                         LLVMValueRef Instr) {
+  auto *IRB = unwrap(Builder);
+  DebugLoc DL = IRB->getUnknownDebugLocation(
+      unwrap(Block), ((Instruction *)unwrap(Instr))->getIterator());
+  if (DL)
+    return wrap(MetadataAsValue::get(IRB->getContext(), DL.getAsMDNode()));
+  return nullptr;
+}
+
+LLVMValueRef LLVMGetUnknownDebugLocationBefore(LLVMBuilderRef Builder,
+                                               LLVMValueRef Instr) {
+  auto *IRB = unwrap(Builder);
+  DebugLoc DL = IRB->getUnknownDebugLocation((Instruction *)unwrap(Instr));
+  if (DL)
+    return wrap(MetadataAsValue::get(IRB->getContext(), DL.getAsMDNode()));
+  return nullptr;
 }
 
 LLVMBasicBlockRef LLVMGetInsertBlock(LLVMBuilderRef Builder) {
    return wrap(unwrap(Builder)->GetInsertBlock());
 }
 
 void LLVMClearInsertionPosition(LLVMBuilderRef Builder) {
   unwrap(Builder)->ClearInsertionPoint();
 }
 
 void LLVMInsertIntoBuilder(LLVMBuilderRef Builder, LLVMValueRef Instr) {
   unwrap(Builder)->Insert(unwrap<Instruction>(Instr));
 }
 
 void LLVMInsertIntoBuilderWithName(LLVMBuilderRef Builder, LLVMValueRef Instr,
                                    const char *Name) {
   unwrap(Builder)->Insert(unwrap<Instruction>(Instr), Name);
 }
 
 void LLVMDisposeBuilder(LLVMBuilderRef Builder) {
   delete unwrap(Builder);
 }
 
 /*--.. Metadata builders ...................................................--*/
 
 void LLVMSetCurrentDebugLocation(LLVMBuilderRef Builder, LLVMValueRef L) {
-  MDNode *Loc =
-      L ? cast<MDNode>(unwrap<MetadataAsValue>(L)->getMetadata()) : nullptr;
-  unwrap(Builder)->SetCurrentDebugLocation(DebugLoc(Loc));
+  unwrap(Builder)->SetCurrentDebugLocation(getValueRefAsDebugLoc(L));
 }
 
 LLVMValueRef LLVMGetCurrentDebugLocation(LLVMBuilderRef Builder) {
   LLVMContext &Context = unwrap(Builder)->getContext();
   return wrap(MetadataAsValue::get(
       Context, unwrap(Builder)->getCurrentDebugLocation().getAsMDNode()));
 }
 

lib/IR/IRBuilder.cpp

 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Statepoint.h"
 using namespace llvm;
 
+DebugLoc IRBuilderBase::getUnknownDebugLocation(Instruction *InsertPt) {
+  if (InsertPt) {
+    DebugLoc CurrentLoc = InsertPt->getDebugLoc();
+
+    // If the function has debug info, create a zero location in the closest
+    // scope we can find.
+    if (CurrentLoc)
+      return DebugLoc::get(0, 0, CurrentLoc.getInlinedAtScope());
+  }
+
+  // Use an empty location.
+  return DebugLoc();
+}
+
+DebugLoc IRBuilderBase::getUnknownDebugLocation(BasicBlock *ForBB,
+                                                BasicBlock::iterator IP) {
+  if (IP != ForBB->end())
+    return getUnknownDebugLocation(&*IP);
+  return getUnknownDebugLocation((Instruction *)nullptr);
+}
+
 /// CreateGlobalString - Make a new global variable with an initializer that
 /// has array of i8 type filled in with the nul terminated string value
 /// specified.  If Name is specified, it is the name of the global variable
 /// created.
 GlobalVariable *IRBuilderBase::CreateGlobalString(StringRef Str,
                                                   const Twine &Name,
                                                   unsigned AddressSpace) {
   Constant *StrConstant = ConstantDataArray::getString(Context, Str);

tools/llvm-c-test/calc.c

static void handle_line(char **tokens, int ntokens) {
   LLVMModuleRef M = LLVMModuleCreateWithName(name);
 
   LLVMTypeRef I64ty = LLVMInt64Type();
   LLVMTypeRef I64Ptrty = LLVMPointerType(I64ty, 0);
   LLVMTypeRef Fty = LLVMFunctionType(I64ty, &I64Ptrty, 1, 0);
 
   LLVMValueRef F = LLVMAddFunction(M, name, Fty);
   LLVMBuilderRef builder = LLVMCreateBuilder();
-  LLVMPositionBuilderAtEnd(builder, LLVMAppendBasicBlock(F, "entry"));
+  LLVMPositionBuilderAtEndWithLoc(
+      builder, LLVMAppendBasicBlock(F, "entry"),
+      LLVMGetUnknownDebugLocationBefore(builder, NULL));
 
   LLVMGetParams(F, &param);
   LLVMSetValueName(param, "in");
 
   res = build_from_tokens(tokens + 1, ntokens - 1, builder, param);
   if (res) {
     char *irstr = LLVMPrintModuleToString(M);
     puts(irstr);

tools/llvm-c-test/echo.cpp

if (i != VMap.end())
       return i->second;
 
     if (!LLVMIsAInstruction(Src))
       report_fatal_error("Expected an instruction");
 
     auto Ctx = LLVMGetModuleContext(M);
     auto Builder = LLVMCreateBuilderInContext(Ctx);
     auto BB = DeclareBB(LLVMGetInstructionParent(Src));
-    LLVMPositionBuilderAtEnd(Builder, BB);
+    LLVMPositionBuilderAtEndWithLoc(
+        Builder, BB, LLVMGetUnknownDebugLocationBefore(Builder, nullptr));
     auto Dst = CloneInstruction(Src, Builder);
     LLVMDisposeBuilder(Builder);
     return Dst;
   }
 
   void CloneAttrs(LLVMValueRef Src, LLVMValueRef Dst) {
     auto Ctx = LLVMGetModuleContext(M);
     int ArgCount = LLVMGetNumArgOperands(Src);
LLVMBasicBlockRef CloneBB(LLVMBasicBlockRef Src) {
     if (First == nullptr) {
       if (Last != nullptr)
         report_fatal_error("Has no first instruction, but last one");
       return BB;
     }
 
     auto Ctx = LLVMGetModuleContext(M);
     LLVMBuilderRef Builder = LLVMCreateBuilderInContext(Ctx);
-    LLVMPositionBuilderAtEnd(Builder, BB);
+    LLVMPositionBuilderAtEndWithLoc(
+        Builder, BB, LLVMGetUnknownDebugLocationBefore(Builder, nullptr));
 
     LLVMValueRef Cur = First;
     LLVMValueRef Next = nullptr;
     while(true) {
       CloneInstruction(Cur, Builder);
       Next = LLVMGetNextInstruction(Cur);
       if (Next == nullptr) {
         if (Cur != Last)

unittests/IR/IRBuilderTest.cpp

TEST_F(IRBuilderTest, DebugLoc) {
   auto CU = DIB.createCompileUnit(dwarf::DW_LANG_C_plus_plus_11,
                                   DIB.createFile("tmp.cpp", "/"), "", true, "",
                                   0);
   auto SPType = DIB.createSubroutineType(DIB.getOrCreateTypeArray(None));
   auto SP =
       DIB.createFunction(CU, "foo", "foo", File, 1, SPType, false, true, 1);
   DebugLoc DL1 = DILocation::get(Ctx, 2, 0, SP);
   DebugLoc DL2 = DILocation::get(Ctx, 3, 0, SP);
+  DebugLoc DLZero = DILocation::get(Ctx, 0, 0, SP);
 
   auto BB2 = BasicBlock::Create(Ctx, "bb2", F);
   auto Br = BranchInst::Create(BB2, BB);
   Br->setDebugLoc(DL1);
 
   IRBuilder<> Builder(Ctx);
   Builder.SetInsertPoint(Br);
   EXPECT_EQ(DL1, Builder.getCurrentDebugLocation());
   auto Call1 = Builder.CreateCall(Callee, None);
   EXPECT_EQ(DL1, Call1->getDebugLoc());
 
   Call1->setDebugLoc(DL2);
   Builder.SetInsertPoint(Call1->getParent(), Call1->getIterator());
   EXPECT_EQ(DL2, Builder.getCurrentDebugLocation());
   auto Call2 = Builder.CreateCall(Callee, None);
   EXPECT_EQ(DL2, Call2->getDebugLoc());
 
+  // Test the new setInsertPoint() API's.
+  IRBuilder<> Builder2(Ctx);
+  EXPECT_EQ(Builder2.getCurrentDebugLocation(), DebugLoc());
+
+  Builder2.setInsertPoint(BB, DL1);
+  EXPECT_EQ(Builder2.getCurrentDebugLocation(), DL1);
+
+  // Even if there is a current debug location set, getUnknownDebugLocation()
+  // must return an empty location if no insertion point is specified.
+  EXPECT_EQ(Builder2.getUnknownDebugLocation(), DebugLoc());
+
+  Builder2.setInsertPoint(Br, DL2);
+  EXPECT_EQ(Builder2.getCurrentDebugLocation(), DL2);
+
+  Builder2.setInsertPoint(BB, BB->begin(), DL1);
+  EXPECT_EQ(Builder2.getCurrentDebugLocation(), DL1);
+
+  // If the insertion point has debug info, the unknown location should be
+  // <file>:0:0.
+  EXPECT_EQ(Builder2.getUnknownDebugLocation(Br), DLZero);
+  EXPECT_EQ(Builder2.getUnknownDebugLocation(BB, Br->getIterator()), DLZero);
+
+  // If the insertion point has no known debug info, or if the insertion point
+  // is not specified, the unknown location should be empty.
+  Br->setDebugLoc(DebugLoc());
+  EXPECT_EQ(Builder2.getUnknownDebugLocation(), DebugLoc());
+  EXPECT_EQ(Builder2.getUnknownDebugLocation(Br), DebugLoc());
+
   DIB.finalize();
 }
 
 TEST_F(IRBuilderTest, DIImportedEntity) {
   IRBuilder<> Builder(BB);
   DIBuilder DIB(*M);
   auto F = DIB.createFile("F.CBL", "/");
   auto CU = DIB.createCompileUnit(dwarf::DW_LANG_Cobol74,