rework patch

* C: add new versions without bool param
* ocaml: position_before_dbg_records stays the same (equ. to LLVMPositionBuilderBeforeInstrAndDbgRecords)
* ocaml: position_builder2 -> position_builder_before_dbg_records (equ. to LLVMPositionBuilderBeforeDbgRecords)

Author: OCHyams

llvm/bindings/ocaml/llvm/llvm.ml

@@ -1135,8 +1135,9 @@ external delete_instruction : llvalue -> unit = "llvm_delete_instruction"
 external builder : llcontext -> llbuilder = "llvm_builder"
 external position_builder : (llbasicblock, llvalue) llpos -> llbuilder -> unit
                           = "llvm_position_builder"
-external position_builder2 : (llbasicblock, llvalue) llpos -> bool -> llbuilder ->
-                            unit = "llvm_position_builder2"
+external position_builder_before_dbg_records : (llbasicblock, llvalue) llpos ->
+                                               llbuilder -> unit
+                                  = "llvm_position_builder_before_dbg_records"
 external insertion_block : llbuilder -> llbasicblock = "llvm_insertion_block"
 external insert_into_builder : llvalue -> string -> llbuilder -> unit
                              = "llvm_insert_into_builder"
@@ -1150,7 +1151,8 @@ let builder_before context i = builder_at context (Before i)
 let builder_at_end context bb = builder_at context (At_end bb)
 
 let position_before i = position_builder (Before i)
-let position_before_dbg_records i = position_builder2 (Before i) true
+let position_before_dbg_records i =
+  position_builder_before_dbg_records (Before i)
 let position_at_end bb = position_builder (At_end bb)
 
 

llvm/bindings/ocaml/llvm/llvm.mli

@@ -1874,21 +1874,21 @@ val builder_at_end : llcontext -> llbasicblock -> llbuilder
     See the constructor for [llvm::LLVMBuilder]. *)
 val position_builder : (llbasicblock, llvalue) llpos -> llbuilder -> unit
 
-(** [position_builder2 ip bb before_dbg_records] moves the instruction builder
-    [bb] to the position [ip]. [before_dbg_records] determines whether the
-    insertion point is before debug records attached to [ip].
+(** [position_builder_before_dbg_records ip bb before_dbg_records] moves the
+    instruction builder [bb] to the position [ip], before any debug records
+    there.
     See the constructor for [llvm::LLVMBuilder]. *)
-val position_builder2 : (llbasicblock, llvalue) llpos -> bool -> llbuilder ->
-                        unit
+val position_builder_before_dbg_records : (llbasicblock, llvalue) llpos ->
+                                          llbuilder -> unit
 
 (** [position_before ins b] moves the instruction builder [b] to before the
     instruction [isn]. See the method [llvm::LLVMBuilder::SetInsertPoint]. *)
 val position_before : llvalue -> llbuilder -> unit
 
-(** [position_before_dbg_records ins b] moves the instruction builder [b] to
-    before the instruction [isn] and any debug records attached to it.
+(** [position_before_dbg_records ins b] moves the instruction builder [b]
+    to before the instruction [isn] and any debug records attached to it.
     See the method [llvm::LLVMBuilder::SetInsertPoint]. *)
-val position_before : llvalue -> llbuilder -> unit
+val position_before_dbg_records : llvalue -> llbuilder -> unit
 
 (** [position_at_end bb b] moves the instruction builder [b] to the end of the
     basic block [bb]. See the method [llvm::LLVMBuilder::SetInsertPoint]. *)

llvm/bindings/ocaml/llvm/llvm_ocaml.c

@@ -2004,26 +2004,28 @@ value llvm_builder(value C) {
   return alloc_builder(LLVMCreateBuilderInContext(Context_val(C)));
 }
 
-static value llvm_position_builder_impl(value Pos, value B,
-                                        LLVMBool BeforeDbgRecords) {
+/* (llbasicblock, llvalue) llpos -> llbuilder -> unit */
+value llvm_position_builder_before_dbg_records(value Pos, value B) {
   if (Tag_val(Pos) == 0) {
     LLVMBasicBlockRef BB = BasicBlock_val(Field(Pos, 0));
     LLVMPositionBuilderAtEnd(Builder_val(B), BB);
   } else {
     LLVMValueRef I = Value_val(Field(Pos, 0));
-    LLVMPositionBuilderBefore2(Builder_val(B), I, BeforeDbgRecords);
+    LLVMPositionBuilderBeforeInstrAndDbgRecords(Builder_val(B), I);
   }
   return Val_unit;
 }
 
-/* (llbasicblock, llvalue) llpos -> bool -> llbuilder -> unit */
-value llvm_position_builder2(value Pos, value BeforeDbgRecords, value B) {
-  return llvm_position_builder_impl(Pos, B, Bool_val(BeforeDbgRecords));
-}
-
 /* (llbasicblock, llvalue) llpos -> llbuilder -> unit */
 value llvm_position_builder(value Pos, value B) {
-  return llvm_position_builder_impl(Pos, B, /*BeforeDbgRecords=*/0);
+  if (Tag_val(Pos) == 0) {
+    LLVMBasicBlockRef BB = BasicBlock_val(Field(Pos, 0));
+    LLVMPositionBuilderAtEnd(Builder_val(B), BB);
+  } else {
+    LLVMValueRef I = Value_val(Field(Pos, 0));
+    LLVMPositionBuilderBefore(Builder_val(B), I);
+  }
+  return Val_unit;
 }
 
 /* llbuilder -> llbasicblock */

llvm/docs/ReleaseNotes.rst

@@ -214,7 +214,7 @@ Changes to the C API
   * ``LLVMConstICmp``
   * ``LLVMConstFCmp``
 
-* Added ``LLVMPositionBuilder2`` and ``LLVMPositionBuilderBefore2``. Same as ``LLVMPositionBuilder`` and ``LLVMPositionBuilderBefore`` with a new ``BeforeDbgRecords`` parameter which indicates whether or not the insertion position should also be before the debug records that precede the instruction. See the `debug info migration guide <https://llvm.org/docs/RemoveDIsDebugInfo.html>`_ for more info. ``LLVMPositionBuilder`` and ``LLVMPositionBuilderBefore`` are unchanged. They insert before the indicated instruction but after any attached debug records (equivalent to calling the new variants with ``BeforeDbgRecords`` set to ``false``).
+* Added ``LLVMPositionBuilderBeforeDbgRecords`` and ``LLVMPositionBuilderBeforeInstrAndDbgRecords``. Same as ``LLVMPositionBuilder`` and ``LLVMPositionBuilderBefore`` except the insertion position is set to before the debug records that precede the target instruction. See the `debug info migration guide <https://llvm.org/docs/RemoveDIsDebugInfo.html>`_ for more info. ``LLVMPositionBuilder`` and ``LLVMPositionBuilderBefore`` are unchanged; they insert before the indicated instruction but after any attached debug records.
 
 Changes to the CodeGen infrastructure
 -------------------------------------

llvm/docs/RemoveDIsDebugInfo.md

@@ -63,16 +63,16 @@ LLVMDIBuilderInsertDbgValueAtEnd   # Same as above.
 
 New functions (no plans to deprecate)
 ----------------------------------
-LLVMPositionBuilder2               # LLVMPositionBuilder but with BeforeDbgRecords parameter. See info below.
-LLVMPositionBuilderBefore2         # Same as above.
+LLVMPositionBuilderBeforeDbgRecords          # See info below.
+LLVMPositionBuilderBeforeInstrAndDbgRecords  # See info below.
 ```
 
-`LLVMPositionBuilder2` and `LLVMPositionBuilderBefore2` have a parameter `BeforeDbgRecords` which indicates whether or not the insertion position should also be before the debug records that precede the instruction. Note that this doesn't mean that debug intrinsics before the chosen instruction are skipped, only debug records (which unlike debug records are not themselves instructions).
+`LLVMPositionBuilderBeforeDbgRecords` and `LLVMPositionBuilderBeforeInstrAndDbgRecords` behave the same as `LLVMPositionBuilder` and `LLVMPositionBuilderBefore` except the insition position is set before the debug records that precede the target instruction. Note that this doesn't mean that debug intrinsics before the chosen instruction are skipped, only debug records (which unlike debug records are not themselves instructions).
 
-If you don't know whether it should be true or false then follow this rule:
-If you are trying to insert at the start of a block, or purposfully skip debug intrinsics to determine the insertion point for any other reason, then set it to true. Otherwise set it to false.
+If you don't know which function to call then follow this rule:
+If you are trying to insert at the start of a block, or purposfully skip debug intrinsics to determine the insertion point for any other reason, then call the new functions.
 
-`LLVMPositionBuilder` and `LLVMPositionBuilderBefore` are unchanged. They insert before the indicated instruction but after any attached debug records (equivalent to calling the new variants with `BeforeDbgRecords` set to `false`).
+`LLVMPositionBuilder` and `LLVMPositionBuilderBefore` are unchanged. They insert before the indicated instruction but after any attached debug records.
 
 # The new "Debug Record" model
 

llvm/include/llvm-c/Core.h

@@ -3962,11 +3962,12 @@ LLVMBuilderRef LLVMCreateBuilderInContext(LLVMContextRef C);
 LLVMBuilderRef LLVMCreateBuilder(void);
 void LLVMPositionBuilder(LLVMBuilderRef Builder, LLVMBasicBlockRef Block,
                          LLVMValueRef Instr);
-void LLVMPositionBuilder2(LLVMBuilderRef Builder, LLVMBasicBlockRef Block,
-                          LLVMValueRef Instr, LLVMBool BeforeDbgRecords);
+void LLVMPositionBuilderBeforeDbgRecords(LLVMBuilderRef Builder,
+                                         LLVMBasicBlockRef Block,
+                                         LLVMValueRef Inst);
 void LLVMPositionBuilderBefore(LLVMBuilderRef Builder, LLVMValueRef Instr);
-void LLVMPositionBuilderBefore2(LLVMBuilderRef Builder, LLVMValueRef Instr,
-                                LLVMBool BeforeDbgRecords);
+void LLVMPositionBuilderBeforeInstrAndDbgRecords(LLVMBuilderRef Builder,
+                                                 LLVMValueRef Instr);
 void LLVMPositionBuilderAtEnd(LLVMBuilderRef Builder, LLVMBasicBlockRef Block);
 LLVMBasicBlockRef LLVMGetInsertBlock(LLVMBuilderRef Builder);
 void LLVMClearInsertionPosition(LLVMBuilderRef Builder);

llvm/lib/IR/Core.cpp

@@ -3137,29 +3137,35 @@ LLVMBuilderRef LLVMCreateBuilder(void) {
   return LLVMCreateBuilderInContext(LLVMGetGlobalContext());
 }
 
+static void LLVMPositionBuilderImpl(IRBuilder<> *Builder, BasicBlock *Block,
+                                    Instruction *Instr, bool BeforeDbgRecords) {
+  BasicBlock::iterator I = Instr ? Instr->getIterator() : Block->end();
+  I.setHeadBit(BeforeDbgRecords);
+  Builder->SetInsertPoint(Block, I);
+}
+
 void LLVMPositionBuilder(LLVMBuilderRef Builder, LLVMBasicBlockRef Block,
                          LLVMValueRef Instr) {
-  return LLVMPositionBuilder2(Builder, Block, Instr, false);
+  return LLVMPositionBuilderImpl(unwrap(Builder), unwrap(Block),
+                                 unwrap<Instruction>(Instr), false);
 }
 
-void LLVMPositionBuilder2(LLVMBuilderRef Builder, LLVMBasicBlockRef Block,
-                          LLVMValueRef Instr, LLVMBool BeforeDbgRecords) {
-  BasicBlock *BB = unwrap(Block);
-  auto I = Instr ? unwrap<Instruction>(Instr)->getIterator() : BB->end();
-  I.setHeadBit(BeforeDbgRecords);
-  unwrap(Builder)->SetInsertPoint(BB, I);
+void LLVMPositionBuilderBeforeDbgRecords(LLVMBuilderRef Builder,
+                                         LLVMBasicBlockRef Block,
+                                         LLVMValueRef Instr) {
+  return LLVMPositionBuilderImpl(unwrap(Builder), unwrap(Block),
+                                 unwrap<Instruction>(Instr), true);
 }
 
 void LLVMPositionBuilderBefore(LLVMBuilderRef Builder, LLVMValueRef Instr) {
-  return LLVMPositionBuilderBefore2(Builder, Instr, false);
+  Instruction *I = unwrap<Instruction>(Instr);
+  return LLVMPositionBuilderImpl(unwrap(Builder), I->getParent(), I, false);
 }
 
-void LLVMPositionBuilderBefore2(LLVMBuilderRef Builder, LLVMValueRef Instr,
-                                LLVMBool BeforeDbgRecords) {
+void LLVMPositionBuilderBeforeInstrAndDbgRecords(LLVMBuilderRef Builder,
+                                                 LLVMValueRef Instr) {
   Instruction *I = unwrap<Instruction>(Instr);
-  BasicBlock::iterator It = I->getIterator();
-  It.setHeadBit(BeforeDbgRecords);
-  unwrap(Builder)->SetInsertPoint(I->getParent(), It);
+  return LLVMPositionBuilderImpl(unwrap(Builder), I->getParent(), I, true);
 }
 
 void LLVMPositionBuilderAtEnd(LLVMBuilderRef Builder, LLVMBasicBlockRef Block) {

llvm/tools/llvm-c-test/debuginfo.c

@@ -235,16 +235,16 @@ int llvm_test_dibuilder(bool NewDebugInfoFormat) {
   // will come before the `phi` (and be absorbed onto it) which is an invalid
   // state.
   LLVMValueRef InsertPos = LLVMGetFirstInstruction(FooVarBlock);
-  LLVMPositionBuilderBefore2(Builder, InsertPos, true);
+  LLVMPositionBuilderBeforeInstrAndDbgRecords(Builder, InsertPos);
   LLVMValueRef Phi1 = LLVMBuildPhi(Builder, I64, "p1");
   LLVMAddIncoming(Phi1, &Zero, &FooEntryBlock, 1);
   // Do the same again using the other position-setting function.
-  LLVMPositionBuilder2(Builder, FooVarBlock, InsertPos, true);
+  LLVMPositionBuilderBeforeDbgRecords(Builder, FooVarBlock, InsertPos);
   LLVMValueRef Phi2 = LLVMBuildPhi(Builder, I64, "p2");
   LLVMAddIncoming(Phi2, &Zero, &FooEntryBlock, 1);
   // Insert a non-phi before the `ret` but not before the debug records to
   // test that works as expected.
-  LLVMPositionBuilder2(Builder, FooVarBlock, Ret, false);
+  LLVMPositionBuilder(Builder, FooVarBlock, Ret);
   LLVMBuildAdd(Builder, Phi1, Phi2, "a");
 
   char *MStr = LLVMPrintModuleToString(M);