Skip to content

[RemoveDIs] Update DIBuilder C API with DbgRecord functions. #95535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 0 additions & 39 deletions llvm/bindings/ocaml/debuginfo/debuginfo_ocaml.c
Original file line number Diff line number Diff line change
Expand Up @@ -969,45 +969,6 @@ value llvm_dibuild_create_parameter_variable_bytecode(value *argv, int arg) {
);
}

value llvm_dibuild_insert_declare_before_native(value Builder, value Storage,
value VarInfo, value Expr,
value DebugLoc, value Instr) {
LLVMDbgRecordRef Value = LLVMDIBuilderInsertDeclareBefore(
DIBuilder_val(Builder), Value_val(Storage), Metadata_val(VarInfo),
Metadata_val(Expr), Metadata_val(DebugLoc), Value_val(Instr));
return to_val(Value);
}

value llvm_dibuild_insert_declare_before_bytecode(value *argv, int arg) {

return llvm_dibuild_insert_declare_before_native(argv[0], // Builder
argv[1], // Storage
argv[2], // VarInfo
argv[3], // Expr
argv[4], // DebugLoc
argv[5] // Instr
);
}

value llvm_dibuild_insert_declare_at_end_native(value Builder, value Storage,
value VarInfo, value Expr,
value DebugLoc, value Block) {
LLVMDbgRecordRef Value = LLVMDIBuilderInsertDeclareAtEnd(
DIBuilder_val(Builder), Value_val(Storage), Metadata_val(VarInfo),
Metadata_val(Expr), Metadata_val(DebugLoc), BasicBlock_val(Block));
return to_val(Value);
}

value llvm_dibuild_insert_declare_at_end_bytecode(value *argv, int arg) {
return llvm_dibuild_insert_declare_at_end_native(argv[0], // Builder
argv[1], // Storage
argv[2], // VarInfo
argv[3], // Expr
argv[4], // DebugLoc
argv[5] // Block
);
}

value llvm_dibuild_expression(value Builder, value Addr) {
return to_val(LLVMDIBuilderCreateExpression(
DIBuilder_val(Builder), (uint64_t *)Op_val(Addr), Wosize_val(Addr)));
Expand Down
20 changes: 0 additions & 20 deletions llvm/bindings/ocaml/debuginfo/llvm_debuginfo.ml
Original file line number Diff line number Diff line change
Expand Up @@ -592,26 +592,6 @@ external dibuild_create_parameter_variable :
Llvm.llmetadata
= "llvm_dibuild_create_parameter_variable_bytecode" "llvm_dibuild_create_parameter_variable_native"

external dibuild_insert_declare_before :
lldibuilder ->
storage:Llvm.llvalue ->
var_info:Llvm.llmetadata ->
expr:Llvm.llmetadata ->
location:Llvm.llmetadata ->
instr:Llvm.llvalue ->
Llvm.lldbgrecord
= "llvm_dibuild_insert_declare_before_bytecode" "llvm_dibuild_insert_declare_before_native"

external dibuild_insert_declare_at_end :
lldibuilder ->
storage:Llvm.llvalue ->
var_info:Llvm.llmetadata ->
expr:Llvm.llmetadata ->
location:Llvm.llmetadata ->
block:Llvm.llbasicblock ->
Llvm.lldbgrecord
= "llvm_dibuild_insert_declare_at_end_bytecode" "llvm_dibuild_insert_declare_at_end_native"

Copy link
Contributor

@OCHyams OCHyams Jun 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments above from debuginfo_ocml.c - I don't think these should be deleted

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restoring the deleted functions.

external dibuild_expression :
lldibuilder ->
Int64.t array ->
Expand Down
24 changes: 0 additions & 24 deletions llvm/bindings/ocaml/debuginfo/llvm_debuginfo.mli
Original file line number Diff line number Diff line change
Expand Up @@ -652,30 +652,6 @@ val dibuild_create_parameter_variable :
(** [dibuild_create_parameter_variable] Create a new descriptor for a
function parameter variable. *)

val dibuild_insert_declare_before :
lldibuilder ->
storage:Llvm.llvalue ->
var_info:Llvm.llmetadata ->
expr:Llvm.llmetadata ->
location:Llvm.llmetadata ->
instr:Llvm.llvalue ->
Llvm.lldbgrecord
(** [dibuild_insert_declare_before] Insert a new llvm.dbg.declare
intrinsic call before the given instruction [instr]. *)

val dibuild_insert_declare_at_end :
lldibuilder ->
storage:Llvm.llvalue ->
var_info:Llvm.llmetadata ->
expr:Llvm.llmetadata ->
location:Llvm.llmetadata ->
block:Llvm.llbasicblock ->
Llvm.lldbgrecord
(** [dibuild_insert_declare_at_end] Insert a new llvm.dbg.declare
intrinsic call at the end of basic block [block]. If [block]
has a terminator instruction, the intrinsic is inserted
before that terminator instruction. *)

val dibuild_expression : lldibuilder -> Int64.t array -> Llvm.llmetadata
(** [dibuild_expression] Create a new descriptor for the specified variable
which has a complex address expression for its address.
Expand Down
38 changes: 37 additions & 1 deletion llvm/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,43 @@ 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.
* Added the following functions to insert before the indicated instruction but
after any attached debug records.

* ``LLVMPositionBuilderBeforeDbgRecords``
* ``LLVMPositionBuilderBeforeInstrAndDbgRecords``

Same as ``LLVMPositionBuilder`` and ``LLVMPositionBuilderBefore`` except the
insertion position is set to before the debug records that precede the target
instruction. ``LLVMPositionBuilder`` and ``LLVMPositionBuilderBefore`` are
unchanged.

See the `debug info migration guide <https://llvm.org/docs/RemoveDIsDebugInfo.html>`_ for more info.

* Added the following functions to get/set the new non-instruction debug info format.

* ``LLVMIsNewDbgInfoFormat``
* ``LLVMSetIsNewDbgInfoFormat``

* Added the following functions (no plans to deprecate) to insert a debug record
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"no plans to deprecate" would be the default assumption so I don't think you need to mention it here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

(new debug info format).

* ``LLVMDIBuilderInsertDeclareRecordBefore``
* ``LLVMDIBuilderInsertDeclareRecordAtEnd``
* ``LLVMDIBuilderInsertDbgValueRecordBefore``
* ``LLVMDIBuilderInsertDbgValueRecordAtEnd``

* Deleted the following functions that inserted a debug intrinsic (old debug info format)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: missing full stop.

It's a bit repetitive but we probably want See the debug info migration guide https://llvm.org/docs/RemoveDIsDebugInfo.html_ for more info. included in each one of these descriptive bullet points. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I would suggest is something like this:

Changes to the C API

...
Note: The following changes are due to the removal of the debug info
intrinsics from LLVM and to the introduction of debug records into LLVM.
They are described in detail in the debug info migration guide <https://llvm.org/docs/RemoveDIsDebugInfo.html>_.

  • Added the following functions to insert before the indicated instruction but
    ....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That moves the info further away from the place someone would be looking, and requires these changes always sit at the bottom of the list. These are very minor concerns though so I'm happy to go with whatever you prefer here (don't forget the full stop).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pogo59, @nikic any opinion on this? Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These notes are for a group of related changes. The "Note" seems okay to me. Another tactic would be to put this group of changes into a new subsection, which could help other release-note writers realize what's going on. That said, C API changes aren't super common and I'm equally fine with just having the Note.


* ``LLVMDIBuilderInsertDeclareBefore``
* ``LLVMDIBuilderInsertDeclareAtEnd``
* ``LLVMDIBuilderInsertDbgValueBefore``
* ``LLVMDIBuilderInsertDbgValueAtEnd``

* ``LLVMDIBuilderInsertDeclareIntrinsicBefore``
* ``LLVMDIBuilderInsertDeclareIntrinsicAtEnd``
* ``LLVMDIBuilderInsertDbgValueIntrinsicBefore``
* ``LLVMDIBuilderInsertDbgValueIntrinsicAtEnd``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to mention these as these were ones I added that we're removing, they are not part of the original C API.


Changes to the CodeGen infrastructure
-------------------------------------
Expand Down
32 changes: 17 additions & 15 deletions llvm/docs/RemoveDIsDebugInfo.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,34 +140,36 @@ Any tests downstream of the main LLVM repo that test the IR output of LLVM may b
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)
------------------------------------
LLVMIsNewDbgInfoFormat # Returns true if the module is in the new non-instruction mode.
LLVMSetIsNewDbgInfoFormat # Convert to the requested debug info format.
Deleted functions
-----------------
LLVMDIBuilderInsertDeclareBefore # Insert a debug record (new debug info format) instead of a debug intrinsic (old debug info format).
LLVMDIBuilderInsertDeclareAtEnd # Same as above.
LLVMDIBuilderInsertDbgValueBefore # Same as above.
LLVMDIBuilderInsertDbgValueAtEnd # Same as above.

LLVMDIBuilderInsertDeclareIntrinsicBefore # Insert a debug intrinsic (old debug info format).
LLVMDIBuilderInsertDeclareIntrinsicAtEnd # Same as above.
LLVMDIBuilderInsertDbgValueIntrinsicBefore # Same as above.
LLVMDIBuilderInsertDbgValueIntrinsicAtEnd # Same as above.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before - these ones don't need to be mentioned at all.


LLVMDIBuilderInsertDeclareRecordBefore # Insert a debug record (new debug info format).
LLVMDIBuilderInsertDeclareRecordAtEnd # Same as above.
LLVMDIBuilderInsertDbgValueRecordBefore # Same as above.
LLVMDIBuilderInsertDbgValueRecordAtEnd # Same as above.
New functions (to be deprecated)
--------------------------------
LLVMIsNewDbgInfoFormat # Returns true if the module is in the new non-instruction mode.
LLVMSetIsNewDbgInfoFormat # Convert to the requested debug info format.

Existing functions (behaviour change)
New functions (no plans to deprecate)
-------------------------------------
LLVMDIBuilderInsertDeclareBefore # Insert a debug record (new debug info format) instead of a debug intrinsic (old debug info format).
LLVMDIBuilderInsertDeclareAtEnd # Same as above.
LLVMDIBuilderInsertDbgValueBefore # Same as above.
LLVMDIBuilderInsertDbgValueAtEnd # Same as above.
LLVMDIBuilderInsertDeclareRecordBefore # Insert a debug record (new debug info format).
LLVMDIBuilderInsertDeclareRecordAtEnd # Same as above. See info below.
LLVMDIBuilderInsertDbgValueRecordBefore # Same as above. See info below.
LLVMDIBuilderInsertDbgValueRecordAtEnd # Same as above. See info below.

New functions (no plans to deprecate)
----------------------------------
LLVMPositionBuilderBeforeDbgRecords # See info below.
LLVMPositionBuilderBeforeInstrAndDbgRecords # See info below.
```

`LLVMDIBuilderInsertDeclareRecordBefore`, `LLVMDIBuilderInsertDeclareRecordAtEnd`, `LLVMDIBuilderInsertDbgValueRecordBefore` and `LLVMDIBuilderInsertDbgValueRecordAtEnd` are replacing the deleted `LLVMDIBuilderInsertDeclareBefore-style` functions.

`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:
Expand Down
142 changes: 0 additions & 142 deletions llvm/include/llvm-c/DebugInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -1261,40 +1261,6 @@ LLVMMetadataRef LLVMDIBuilderCreateTempGlobalVariableFwdDecl(
unsigned LineNo, LLVMMetadataRef Ty, LLVMBool LocalToUnit,
LLVMMetadataRef Decl, uint32_t AlignInBits);

/*
* Insert a new Declare DbgRecord before the given instruction.
*
* Only use in "new debug mode" (LLVMIsNewDbgInfoFormat() is true).
* Use LLVMSetIsNewDbgInfoFormat(LLVMBool) to convert between formats.
* See https://llvm.org/docs/RemoveDIsDebugInfo.html#c-api-changes
*
* \param Builder The DIBuilder.
* \param Storage The storage of the variable to declare.
* \param VarInfo The variable's debug info descriptor.
* \param Expr A complex location expression for the variable.
* \param DebugLoc Debug info location.
* \param Instr Instruction acting as a location for the new intrinsic.
*/
LLVMDbgRecordRef
LLVMDIBuilderInsertDeclareBefore(LLVMDIBuilderRef Builder, LLVMValueRef Storage,
LLVMMetadataRef VarInfo, LLVMMetadataRef Expr,
LLVMMetadataRef DebugLoc, LLVMValueRef Instr);
/**
* Soon to be deprecated.
* Only use in "old debug mode" (LLVMIsNewDbgInfoFormat() is false).
* See https://llvm.org/docs/RemoveDIsDebugInfo.html#c-api-changes
*
* Insert a new llvm.dbg.declare intrinsic call before the given instruction.
* \param Builder The DIBuilder.
* \param Storage The storage of the variable to declare.
* \param VarInfo The variable's debug info descriptor.
* \param Expr A complex location expression for the variable.
* \param DebugLoc Debug info location.
* \param Instr Instruction acting as a location for the new intrinsic.
*/
LLVMValueRef LLVMDIBuilderInsertDeclareIntrinsicBefore(
LLVMDIBuilderRef Builder, LLVMValueRef Storage, LLVMMetadataRef VarInfo,
LLVMMetadataRef Expr, LLVMMetadataRef DebugLoc, LLVMValueRef Instr);
/**
* Soon to be deprecated.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can you remove the Soon to be deprecated notes from all the Record versions? The plan is now to keep the Record versions so these comments are out of date and misleading.

While you're there please could you add another note to the functions indicating that the debug format can be switched later after inserting the records using LLVMSetIsNewDbgInfoFormat, if needed for legacy or transitionary reasons.

Copy link
Member Author

@CarlosAlbertoEnciso CarlosAlbertoEnciso Jun 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete the Soon to be deprecated note.
Added the following note:

 *
 * The debug format can be switched later after inserting the records using
 * LLVMSetIsNewDbgInfoFormat, if needed for legacy or transitionary reasons.
 *

* Only use in "new debug mode" (LLVMIsNewDbgInfoFormat() is true).
Expand All @@ -1312,43 +1278,6 @@ LLVMDbgRecordRef LLVMDIBuilderInsertDeclareRecordBefore(
LLVMDIBuilderRef Builder, LLVMValueRef Storage, LLVMMetadataRef VarInfo,
LLVMMetadataRef Expr, LLVMMetadataRef DebugLoc, LLVMValueRef Instr);

/**
* Insert a new Declare DbgRecord at the end of the given basic block. If the
* basic block has a terminator instruction, the intrinsic is inserted before
* that terminator instruction.
*
* Only use in "new debug mode" (LLVMIsNewDbgInfoFormat() is true).
* Use LLVMSetIsNewDbgInfoFormat(LLVMBool) to convert between formats.
* See https://llvm.org/docs/RemoveDIsDebugInfo.html#c-api-changes
*
* \param Builder The DIBuilder.
* \param Storage The storage of the variable to declare.
* \param VarInfo The variable's debug info descriptor.
* \param Expr A complex location expression for the variable.
* \param DebugLoc Debug info location.
* \param Block Basic block acting as a location for the new intrinsic.
*/
LLVMDbgRecordRef LLVMDIBuilderInsertDeclareAtEnd(
LLVMDIBuilderRef Builder, LLVMValueRef Storage, LLVMMetadataRef VarInfo,
LLVMMetadataRef Expr, LLVMMetadataRef DebugLoc, LLVMBasicBlockRef Block);
/**
* Soon to be deprecated.
* Only use in "old debug mode" (LLVMIsNewDbgInfoFormat() is false).
* See https://llvm.org/docs/RemoveDIsDebugInfo.html#c-api-changes
*
* Insert a new llvm.dbg.declare intrinsic call at the end of the given basic
* block. If the basic block has a terminator instruction, the intrinsic is
* inserted before that terminator instruction.
* \param Builder The DIBuilder.
* \param Storage The storage of the variable to declare.
* \param VarInfo The variable's debug info descriptor.
* \param Expr A complex location expression for the variable.
* \param DebugLoc Debug info location.
* \param Block Basic block acting as a location for the new intrinsic.
*/
LLVMValueRef LLVMDIBuilderInsertDeclareIntrinsicAtEnd(
LLVMDIBuilderRef Builder, LLVMValueRef Storage, LLVMMetadataRef VarInfo,
LLVMMetadataRef Expr, LLVMMetadataRef DebugLoc, LLVMBasicBlockRef Block);
/**
* Soon to be deprecated.
* Only use in "new debug mode" (LLVMIsNewDbgInfoFormat() is true).
Expand All @@ -1368,40 +1297,6 @@ LLVMDbgRecordRef LLVMDIBuilderInsertDeclareRecordAtEnd(
LLVMDIBuilderRef Builder, LLVMValueRef Storage, LLVMMetadataRef VarInfo,
LLVMMetadataRef Expr, LLVMMetadataRef DebugLoc, LLVMBasicBlockRef Block);

/**
* Insert a new Value DbgRecord before the given instruction.
*
* Only use in "new debug mode" (LLVMIsNewDbgInfoFormat() is true).
* Use LLVMSetIsNewDbgInfoFormat(LLVMBool) to convert between formats.
* See https://llvm.org/docs/RemoveDIsDebugInfo.html#c-api-changes
*
* \param Builder The DIBuilder.
* \param Val The value of the variable.
* \param VarInfo The variable's debug info descriptor.
* \param Expr A complex location expression for the variable.
* \param DebugLoc Debug info location.
* \param Instr Instruction acting as a location for the new intrinsic.
*/
LLVMDbgRecordRef
LLVMDIBuilderInsertDbgValueBefore(LLVMDIBuilderRef Builder, LLVMValueRef Val,
LLVMMetadataRef VarInfo, LLVMMetadataRef Expr,
LLVMMetadataRef DebugLoc, LLVMValueRef Instr);
/**
* Soon to be deprecated.
* Only use in "old debug mode" (LLVMIsNewDbgInfoFormat() is false).
* See https://llvm.org/docs/RemoveDIsDebugInfo.html#c-api-changes
*
* Insert a new llvm.dbg.value intrinsic call before the given instruction.
* \param Builder The DIBuilder.
* \param Val The value of the variable.
* \param VarInfo The variable's debug info descriptor.
* \param Expr A complex location expression for the variable.
* \param DebugLoc Debug info location.
* \param Instr Instruction acting as a location for the new intrinsic.
*/
LLVMValueRef LLVMDIBuilderInsertDbgValueIntrinsicBefore(
LLVMDIBuilderRef Builder, LLVMValueRef Val, LLVMMetadataRef VarInfo,
LLVMMetadataRef Expr, LLVMMetadataRef DebugLoc, LLVMValueRef Instr);
/**
* Soon to be deprecated.
* Only use in "new debug mode" (LLVMIsNewDbgInfoFormat() is true).
Expand All @@ -1419,43 +1314,6 @@ LLVMDbgRecordRef LLVMDIBuilderInsertDbgValueRecordBefore(
LLVMDIBuilderRef Builder, LLVMValueRef Val, LLVMMetadataRef VarInfo,
LLVMMetadataRef Expr, LLVMMetadataRef DebugLoc, LLVMValueRef Instr);

/**
* Insert a new Value DbgRecord at the end of the given basic block. If the
* basic block has a terminator instruction, the intrinsic is inserted before
* that terminator instruction.
*
* Only use in "new debug mode" (LLVMIsNewDbgInfoFormat() is true).
* Use LLVMSetIsNewDbgInfoFormat(LLVMBool) to convert between formats.
* See https://llvm.org/docs/RemoveDIsDebugInfo.html#c-api-changes
*
* \param Builder The DIBuilder.
* \param Val The value of the variable.
* \param VarInfo The variable's debug info descriptor.
* \param Expr A complex location expression for the variable.
* \param DebugLoc Debug info location.
* \param Block Basic block acting as a location for the new intrinsic.
*/
LLVMDbgRecordRef LLVMDIBuilderInsertDbgValueAtEnd(
LLVMDIBuilderRef Builder, LLVMValueRef Val, LLVMMetadataRef VarInfo,
LLVMMetadataRef Expr, LLVMMetadataRef DebugLoc, LLVMBasicBlockRef Block);
/**
* Soon to be deprecated.
* Only use in "old debug mode" (LLVMIsNewDbgInfoFormat() is false).
* See https://llvm.org/docs/RemoveDIsDebugInfo.html#c-api-changes
*
* Insert a new llvm.dbg.value intrinsic call at the end of the given basic
* block. If the basic block has a terminator instruction, the intrinsic is
* inserted before that terminator instruction.
* \param Builder The DIBuilder.
* \param Val The value of the variable.
* \param VarInfo The variable's debug info descriptor.
* \param Expr A complex location expression for the variable.
* \param DebugLoc Debug info location.
* \param Block Basic block acting as a location for the new intrinsic.
*/
LLVMValueRef LLVMDIBuilderInsertDbgValueIntrinsicAtEnd(
LLVMDIBuilderRef Builder, LLVMValueRef Val, LLVMMetadataRef VarInfo,
LLVMMetadataRef Expr, LLVMMetadataRef DebugLoc, LLVMBasicBlockRef Block);
/**
* Soon to be deprecated.
* Only use in "new debug mode" (LLVMIsNewDbgInfoFormat() is true).
Expand Down
Loading
Loading