[llvm] d732a32 - [RemoveDIs] C API: Add before-dbg-record versions of IRBuilder position funcs (#92417)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 10 01:21:43 PDT 2024
Author: Orlando Cazalet-Hyams
Date: 2024-06-10T09:21:40+01:00
New Revision: d732a3298a70d83e5571c594de9be63df85668a7
URL: https://github.com/llvm/llvm-project/commit/d732a3298a70d83e5571c594de9be63df85668a7
DIFF: https://github.com/llvm/llvm-project/commit/d732a3298a70d83e5571c594de9be63df85668a7.diff
LOG: [RemoveDIs] C API: Add before-dbg-record versions of IRBuilder position funcs (#92417)
Add `LLVMPositionBuilderBeforeDbgRecords` and
`LLVMPositionBuilderBeforeInstrAndDbgRecords` to `llvm/include/llvm-c/Core.h`
which behave the same as `LLVMPositionBuilder` and `LVMPositionBuilderBefore`
except that the position is set before debug records attached to the target
instruction (the existing functions set the insertion point to after any
attached debug records).
More info on debug records and the migration towards using them can be found
here: https://llvm.org/docs/RemoveDIsDebugInfo.html
The distinction is important in some situations. An important example is when
inserting a phi before another instruction which has debug records attached to
it (these come "before" the instruction). Inserting before the instruction but
after the debug records would result in having debug records before a phi, which
is illegal. That results in an assertion failure:
`llvm/lib/IR/Instruction.cpp:166: Assertion '!isa<PHINode>(this) && "Inserting
PHI after debug-records!"' failed.`
In llvm (C++) we've added bit to instruction iterators that carries around the
extra information. Adding dedicated functions seemed like the least invasive and
least suprising way to update the C API.
Update llvm/tools/llvm-c-test/debuginfo.c to test this functionality.
Update the OCaml bindings, the migration docs and release notes.
Added:
Modified:
llvm/bindings/ocaml/llvm/llvm.ml
llvm/bindings/ocaml/llvm/llvm.mli
llvm/bindings/ocaml/llvm/llvm_ocaml.c
llvm/docs/ReleaseNotes.rst
llvm/docs/RemoveDIsDebugInfo.md
llvm/include/llvm-c/Core.h
llvm/lib/IR/Core.cpp
llvm/test/Bindings/OCaml/core.ml
llvm/test/Bindings/llvm-c/debug_info.ll
llvm/test/Bindings/llvm-c/debug_info_new_format.ll
llvm/tools/llvm-c-test/debuginfo.c
Removed:
################################################################################
diff --git a/llvm/bindings/ocaml/llvm/llvm.ml b/llvm/bindings/ocaml/llvm/llvm.ml
index 7eeaae49e1059..7bfaf8656ab3f 100644
--- a/llvm/bindings/ocaml/llvm/llvm.ml
+++ b/llvm/bindings/ocaml/llvm/llvm.ml
@@ -1135,6 +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_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"
@@ -1148,6 +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_builder_before_dbg_records (Before i)
let position_at_end bb = position_builder (At_end bb)
diff --git a/llvm/bindings/ocaml/llvm/llvm.mli b/llvm/bindings/ocaml/llvm/llvm.mli
index 36cc095dc66e3..89b894b35849d 100644
--- a/llvm/bindings/ocaml/llvm/llvm.mli
+++ b/llvm/bindings/ocaml/llvm/llvm.mli
@@ -1874,10 +1874,22 @@ val builder_at_end : llcontext -> llbasicblock -> llbuilder
See the constructor for [llvm::LLVMBuilder]. *)
val position_builder : (llbasicblock, llvalue) llpos -> llbuilder -> unit
+(** [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_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.
+ See the method [llvm::LLVMBuilder::SetInsertPoint]. *)
+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]. *)
val position_at_end : llbasicblock -> llbuilder -> unit
diff --git a/llvm/bindings/ocaml/llvm/llvm_ocaml.c b/llvm/bindings/ocaml/llvm/llvm_ocaml.c
index 26a3ac21501d2..3976a96a6da9b 100644
--- a/llvm/bindings/ocaml/llvm/llvm_ocaml.c
+++ b/llvm/bindings/ocaml/llvm/llvm_ocaml.c
@@ -2004,6 +2004,18 @@ value llvm_builder(value C) {
return alloc_builder(LLVMCreateBuilderInContext(Context_val(C)));
}
+/* (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));
+ LLVMPositionBuilderBeforeInstrAndDbgRecords(Builder_val(B), I);
+ }
+ return Val_unit;
+}
+
/* (llbasicblock, llvalue) llpos -> llbuilder -> unit */
value llvm_position_builder(value Pos, value B) {
if (Tag_val(Pos) == 0) {
diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index e1e1652d1e0a1..f86f9430201f8 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -214,6 +214,8 @@ Changes to the C API
* ``LLVMConstICmp``
* ``LLVMConstFCmp``
+* 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
-------------------------------------
diff --git a/llvm/docs/RemoveDIsDebugInfo.md b/llvm/docs/RemoveDIsDebugInfo.md
index 3cdf63c63b5a0..ef11c8ed8b433 100644
--- a/llvm/docs/RemoveDIsDebugInfo.md
+++ b/llvm/docs/RemoveDIsDebugInfo.md
@@ -36,7 +36,7 @@ For a more in-depth overview of how to update existing code to support debug rec
# C-API changes
-All the functions that have been added are temporary and will be deprecated in the future. The intention is that they'll help downstream projects adapt during the transition period.
+Some new functions that have been added are temporary and will be deprecated in the future. The intention is that they'll help downstream projects adapt during the transition period.
```
New functions (all to be deprecated)
@@ -60,8 +60,20 @@ LLVMDIBuilderInsertDeclareBefore # Insert a debug record (new debug info forma
LLVMDIBuilderInsertDeclareAtEnd # Same as above.
LLVMDIBuilderInsertDbgValueBefore # Same as above.
LLVMDIBuilderInsertDbgValueAtEnd # Same as above.
+
+New functions (no plans to deprecate)
+----------------------------------
+LLVMPositionBuilderBeforeDbgRecords # See info below.
+LLVMPositionBuilderBeforeInstrAndDbgRecords # See info below.
```
+`LLVMPositionBuilderBeforeDbgRecords` and `LLVMPositionBuilderBeforeInstrAndDbgRecords` behave the same as `LLVMPositionBuilder` and `LLVMPositionBuilderBefore` except the insertion 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 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.
+
# The new "Debug Record" model
Below is a brief overview of the new representation that replaces debug intrinsics; for an instructive guide on updating old code, see [here](#how-to-update-existing-code).
diff --git a/llvm/include/llvm-c/Core.h b/llvm/include/llvm-c/Core.h
index 7d6b8c7607974..cd9d7e0f843d0 100644
--- a/llvm/include/llvm-c/Core.h
+++ b/llvm/include/llvm-c/Core.h
@@ -3951,9 +3951,28 @@ const unsigned *LLVMGetIndices(LLVMValueRef Inst);
LLVMBuilderRef LLVMCreateBuilderInContext(LLVMContextRef C);
LLVMBuilderRef LLVMCreateBuilder(void);
+/**
+ * Set the builder poisiton before Instr but after any attached debug records,
+ * or if Instr is null set the position to the end of Block.
+ */
void LLVMPositionBuilder(LLVMBuilderRef Builder, LLVMBasicBlockRef Block,
LLVMValueRef Instr);
+/**
+ * Set the builder poisiton before Instr and any attached debug records,
+ * or if Instr is null set the position to the end of Block.
+ */
+void LLVMPositionBuilderBeforeDbgRecords(LLVMBuilderRef Builder,
+ LLVMBasicBlockRef Block,
+ LLVMValueRef Inst);
+/**
+ * Set the builder poisiton before Instr but after any attached debug records.
+ */
void LLVMPositionBuilderBefore(LLVMBuilderRef Builder, LLVMValueRef Instr);
+/**
+ * Set the builder poisiton before Instr and any attached debug records.
+ */
+void LLVMPositionBuilderBeforeInstrAndDbgRecords(LLVMBuilderRef Builder,
+ LLVMValueRef Instr);
void LLVMPositionBuilderAtEnd(LLVMBuilderRef Builder, LLVMBasicBlockRef Block);
LLVMBasicBlockRef LLVMGetInsertBlock(LLVMBuilderRef Builder);
void LLVMClearInsertionPosition(LLVMBuilderRef Builder);
diff --git a/llvm/lib/IR/Core.cpp b/llvm/lib/IR/Core.cpp
index 4a508a022cf7e..3a91a4eb6e0d1 100644
--- a/llvm/lib/IR/Core.cpp
+++ b/llvm/lib/IR/Core.cpp
@@ -3137,16 +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) {
- BasicBlock *BB = unwrap(Block);
- auto I = Instr ? unwrap<Instruction>(Instr)->getIterator() : BB->end();
- unwrap(Builder)->SetInsertPoint(BB, I);
+ return LLVMPositionBuilderImpl(unwrap(Builder), unwrap(Block),
+ unwrap<Instruction>(Instr), false);
+}
+
+void LLVMPositionBuilderBeforeDbgRecords(LLVMBuilderRef Builder,
+ LLVMBasicBlockRef Block,
+ LLVMValueRef Instr) {
+ return LLVMPositionBuilderImpl(unwrap(Builder), unwrap(Block),
+ unwrap<Instruction>(Instr), true);
}
void LLVMPositionBuilderBefore(LLVMBuilderRef Builder, LLVMValueRef Instr) {
Instruction *I = unwrap<Instruction>(Instr);
- unwrap(Builder)->SetInsertPoint(I->getParent(), I->getIterator());
+ return LLVMPositionBuilderImpl(unwrap(Builder), I->getParent(), I, false);
+}
+
+void LLVMPositionBuilderBeforeInstrAndDbgRecords(LLVMBuilderRef Builder,
+ LLVMValueRef Instr) {
+ Instruction *I = unwrap<Instruction>(Instr);
+ return LLVMPositionBuilderImpl(unwrap(Builder), I->getParent(), I, true);
}
void LLVMPositionBuilderAtEnd(LLVMBuilderRef Builder, LLVMBasicBlockRef Block) {
diff --git a/llvm/test/Bindings/OCaml/core.ml b/llvm/test/Bindings/OCaml/core.ml
index 36f4c468d99f4..923a3541e239c 100644
--- a/llvm/test/Bindings/OCaml/core.ml
+++ b/llvm/test/Bindings/OCaml/core.ml
@@ -1150,7 +1150,7 @@ let test_builder () =
(* CHECK: ret{{.*}}P1
*)
let ret = build_ret p1 atentry in
- position_before ret atentry
+ position_before_dbg_records ret atentry
end;
(* see test/Feature/exception.ll *)
diff --git a/llvm/test/Bindings/llvm-c/debug_info.ll b/llvm/test/Bindings/llvm-c/debug_info.ll
index 9358bac59bd21..71986fbb2348f 100644
--- a/llvm/test/Bindings/llvm-c/debug_info.ll
+++ b/llvm/test/Bindings/llvm-c/debug_info.ll
@@ -10,7 +10,10 @@
; CHECK-NEXT: call void @llvm.dbg.declare(metadata i64 0, metadata !40, metadata !DIExpression()), !dbg !43
; CHECK-NEXT: br label %vars
; CHECK: vars:
+; CHECK-NEXT: %p1 = phi i64 [ 0, %entry ]
+; CHECK-NEXT: %p2 = phi i64 [ 0, %entry ]
; CHECK-NEXT: call void @llvm.dbg.value(metadata i64 0, metadata !41, metadata !DIExpression(DW_OP_constu, 0, DW_OP_stack_value)), !dbg !44
+; CHECK-NEXT: %a = add i64 %p1, %p2
; CHECK-NEXT: ret i64 0
; CHECK-NEXT: }
diff --git a/llvm/test/Bindings/llvm-c/debug_info_new_format.ll b/llvm/test/Bindings/llvm-c/debug_info_new_format.ll
index 05b6ef4de9adb..1b6f2c4ea403a 100644
--- a/llvm/test/Bindings/llvm-c/debug_info_new_format.ll
+++ b/llvm/test/Bindings/llvm-c/debug_info_new_format.ll
@@ -11,7 +11,10 @@
; CHECK-NEXT: #dbg_declare(i64 0, !40, !DIExpression(), !43)
; CHECK-NEXT: br label %vars
; CHECK: vars:
+; CHECK-NEXT: %p1 = phi i64 [ 0, %entry ]
+; CHECK-NEXT: %p2 = phi i64 [ 0, %entry ]
; CHECK-NEXT: #dbg_value(i64 0, !41, !DIExpression(DW_OP_constu, 0, DW_OP_stack_value), !44)
+; CHECK-NEXT: %a = add i64 %p1, %p2
; CHECK-NEXT: ret i64 0
; CHECK-NEXT: }
diff --git a/llvm/tools/llvm-c-test/debuginfo.c b/llvm/tools/llvm-c-test/debuginfo.c
index 35c65f885af32..032657203b5f2 100644
--- a/llvm/tools/llvm-c-test/debuginfo.c
+++ b/llvm/tools/llvm-c-test/debuginfo.c
@@ -228,7 +228,24 @@ int llvm_test_dibuilder(bool NewDebugInfoFormat) {
LLVMPositionBuilderAtEnd(Builder, FooVarBlock);
LLVMTypeRef I64 = LLVMInt64TypeInContext(Ctx);
LLVMValueRef Zero = LLVMConstInt(I64, 0, false);
- LLVMBuildRet(Builder, Zero);
+ LLVMValueRef Ret = LLVMBuildRet(Builder, Zero);
+
+ // Insert a `phi` before the `ret`. In the new debug info mode we need to
+ // be careful to insert before debug records too, else the debug records
+ // will come before the `phi` (and be absorbed onto it) which is an invalid
+ // state.
+ LLVMValueRef InsertPos = LLVMGetFirstInstruction(FooVarBlock);
+ LLVMPositionBuilderBeforeInstrAndDbgRecords(Builder, InsertPos);
+ LLVMValueRef Phi1 = LLVMBuildPhi(Builder, I64, "p1");
+ LLVMAddIncoming(Phi1, &Zero, &FooEntryBlock, 1);
+ // Do the same again using the other position-setting function.
+ 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.
+ LLVMPositionBuilder(Builder, FooVarBlock, Ret);
+ LLVMBuildAdd(Builder, Phi1, Phi2, "a");
char *MStr = LLVMPrintModuleToString(M);
puts(MStr);
More information about the llvm-commits
mailing list