-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[MLIR][LLVM] Debug info: import debug records directly #167812
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
[MLIR][LLVM] Debug info: import debug records directly #167812
Conversation
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-llvm Author: Bruno Cardoso Lopes (bcardosolopes) ChangesEffectively means we don't need to call into Patch is 23.30 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/167812.diff 4 Files Affected:
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index 09d819a05618b..50cca5d9f48da 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -163,9 +163,10 @@ class ModuleImport {
/// Converts `value` to a float attribute. Asserts if the matching fails.
FloatAttr matchFloatAttr(llvm::Value *value);
- /// Converts `value` to a local variable attribute. Asserts if the matching
- /// fails.
- DILocalVariableAttr matchLocalVariableAttr(llvm::Value *value);
+ /// Converts `valOrVariable` to a local variable attribute. Asserts if the
+ /// matching fails.
+ DILocalVariableAttr matchLocalVariableAttr(
+ llvm::PointerUnion<llvm::Value *, llvm::DILocalVariable *> valOrVariable);
/// Converts `value` to a label attribute. Asserts if the matching fails.
DILabelAttr matchLabelAttr(llvm::Value *value);
@@ -281,6 +282,9 @@ class ModuleImport {
/// after the function conversion has finished.
void addDebugIntrinsic(llvm::CallInst *intrinsic);
+ /// Similar to `addDebugIntrinsic`, but for debug records.
+ void addDebugRecord(llvm::DbgRecord *dr);
+
/// Converts the LLVM values for an intrinsic to mixed MLIR values and
/// attributes for LLVM_IntrOpBase. Attributes correspond to LLVM immargs. The
/// list `immArgPositions` contains the positions of immargs on the LLVM
@@ -339,9 +343,26 @@ class ModuleImport {
/// Converts all debug intrinsics in `debugIntrinsics`. Assumes that the
/// function containing the intrinsics has been fully converted to MLIR.
LogicalResult processDebugIntrinsics();
+ /// Converts all debug records in `debugRecords`. Assumes that the
+ /// function containing the record has been fully converted to MLIR.
+ LogicalResult processDebugRecords();
/// Converts a single debug intrinsic.
LogicalResult processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr,
DominanceInfo &domInfo);
+ /// Converts a single debug record.
+ LogicalResult processDebugRecord(llvm::DbgRecord &dr, DominanceInfo &domInfo);
+ /// Process arguments for declare/value operation insertion. `localVarAttr`
+ /// and `localExprAttr` are the attained attributes after importing the debug
+ /// variable and expressions. This also sets the builder insertion point to be
+ /// used by these operations.
+ void processDebugOpArgumentsAndInsertionPt(
+ Location loc, DILocalVariableAttr &localVarAttr,
+ DIExpressionAttr &localExprAttr, Value &locVal, bool hasArgList,
+ bool isKillLocation,
+ llvm::function_ref<FailureOr<Value>()> convertArgOperandToValue,
+ llvm::Value *address,
+ llvm::PointerUnion<llvm::Value *, llvm::DILocalVariable *> variable,
+ llvm::DIExpression *expression, DominanceInfo &domInfo);
/// Converts LLMV IR asm inline call operand's attributes into an array of
/// MLIR attributes to be utilized in `llvm.inline_asm`.
ArrayAttr convertAsmInlineOperandAttrs(const llvm::CallBase &llvmCall);
@@ -485,6 +506,9 @@ class ModuleImport {
/// Function-local list of debug intrinsics that need to be imported after the
/// function conversion has finished.
SetVector<llvm::Instruction *> debugIntrinsics;
+ /// Function-local list of debug records that need to be imported after the
+ /// function conversion has finished.
+ SetVector<llvm::DbgRecord *> debugRecords;
/// Mapping between LLVM alias scope and domain metadata nodes and
/// attributes in the LLVM dialect corresponding to these nodes.
DenseMap<const llvm::MDNode *, Attribute> aliasScopeMapping;
diff --git a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
index 2dd0640f794e5..ba80f6294bd9b 100644
--- a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -30,6 +30,14 @@ void registerFromLLVMIRTranslation() {
llvm::cl::desc("Emit expensive warnings during LLVM IR import "
"(discouraged: testing only!)"),
llvm::cl::init(false));
+ static llvm::cl::opt<bool> convertDebugRecToIntrinsics(
+ "convert-debug-rec-to-intrinsics",
+ llvm::cl::desc("Change the input LLVM module to use old debug intrinsics "
+ "instead of records "
+ "via convertFromNewDbgValues, this happens "
+ "before importing the debug information"
+ "(discouraged: to be removed soon!)"),
+ llvm::cl::init(false));
static llvm::cl::opt<bool> dropDICompositeTypeElements(
"drop-di-composite-type-elements",
llvm::cl::desc(
@@ -69,8 +77,10 @@ void registerFromLLVMIRTranslation() {
if (llvm::verifyModule(*llvmModule, &llvm::errs()))
return nullptr;
- // Debug records are not currently supported in the LLVM IR translator.
- llvmModule->convertFromNewDbgValues();
+ // Now that the translation supports importing debug records directly,
+ // make it the default, but allow the user to override to old behavior.
+ if (!convertDebugRecToIntrinsics)
+ llvmModule->convertFromNewDbgValues();
return translateLLVMIRToModule(
std::move(llvmModule), context, emitExpensiveWarnings,
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index d9891e3168820..5503dec2887ac 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -34,12 +34,14 @@
#include "llvm/ADT/TypeSwitch.h"
#include "llvm/IR/Comdat.h"
#include "llvm/IR/Constants.h"
+#include "llvm/IR/DebugProgramInstruction.h"
#include "llvm/IR/InlineAsm.h"
#include "llvm/IR/InstIterator.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Operator.h"
+#include "llvm/Support/LogicalResult.h"
#include "llvm/Support/ModRef.h"
#include <optional>
@@ -522,6 +524,11 @@ void ModuleImport::addDebugIntrinsic(llvm::CallInst *intrinsic) {
debugIntrinsics.insert(intrinsic);
}
+void ModuleImport::addDebugRecord(llvm::DbgRecord *dr) {
+ if (!debugRecords.contains(dr))
+ debugRecords.insert(dr);
+}
+
static Attribute convertCGProfileModuleFlagValue(ModuleOp mlirModule,
llvm::MDTuple *mdTuple) {
auto getLLVMFunction =
@@ -2003,9 +2010,15 @@ FloatAttr ModuleImport::matchFloatAttr(llvm::Value *value) {
return floatAttr;
}
-DILocalVariableAttr ModuleImport::matchLocalVariableAttr(llvm::Value *value) {
- auto *nodeAsVal = cast<llvm::MetadataAsValue>(value);
- auto *node = cast<llvm::DILocalVariable>(nodeAsVal->getMetadata());
+DILocalVariableAttr ModuleImport::matchLocalVariableAttr(
+ llvm::PointerUnion<llvm::Value *, llvm::DILocalVariable *> valOrVariable) {
+ llvm::DILocalVariable *node = nullptr;
+ if (auto *value = dyn_cast<llvm::Value *>(valOrVariable)) {
+ auto *nodeAsVal = cast<llvm::MetadataAsValue>(value);
+ node = cast<llvm::DILocalVariable>(nodeAsVal->getMetadata());
+ } else {
+ node = cast<llvm::DILocalVariable *>(valOrVariable);
+ }
return debugImporter->translate(node);
}
@@ -2544,6 +2557,12 @@ LogicalResult ModuleImport::processInstruction(llvm::Instruction *inst) {
if (auto *intrinsic = dyn_cast<llvm::IntrinsicInst>(inst))
return convertIntrinsic(intrinsic);
+ // Capture instruction with attached debug markers for later processing.
+ if (inst->DebugMarker) {
+ for (llvm::DbgRecord &dr : inst->DebugMarker->getDbgRecordRange())
+ addDebugRecord(&dr);
+ }
+
// Convert all remaining LLVM instructions to MLIR operations.
return convertInstruction(inst);
}
@@ -3007,76 +3026,50 @@ LogicalResult ModuleImport::processFunction(llvm::Function *func) {
if (failed(processDebugIntrinsics()))
return failure();
+ // Process the debug r that require a delayed conversion after
+ // everything else was converted.
+ if (failed(processDebugRecords()))
+ return failure();
+
return success();
}
-/// Checks if `dbgIntr` is a kill location that holds metadata instead of an SSA
-/// value.
-static bool isMetadataKillLocation(llvm::DbgVariableIntrinsic *dbgIntr) {
- if (!dbgIntr->isKillLocation())
+/// Checks if a kill location holds metadata instead of an SSA value.
+static bool isMetadataKillLocation(bool isKillLocation, llvm::Value *value) {
+ if (!isKillLocation)
return false;
- llvm::Value *value = dbgIntr->getArgOperand(0);
auto *nodeAsVal = dyn_cast<llvm::MetadataAsValue>(value);
if (!nodeAsVal)
return false;
return !isa<llvm::ValueAsMetadata>(nodeAsVal->getMetadata());
}
-LogicalResult
-ModuleImport::processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr,
- DominanceInfo &domInfo) {
- Location loc = translateLoc(dbgIntr->getDebugLoc());
- auto emitUnsupportedWarning = [&]() {
- if (emitExpensiveWarnings)
- emitWarning(loc) << "dropped intrinsic: " << diag(*dbgIntr);
- return success();
- };
- // Drop debug intrinsics with arg lists.
- // TODO: Support debug intrinsics that have arg lists.
- if (dbgIntr->hasArgList())
- return emitUnsupportedWarning();
- // Kill locations can have metadata nodes as location operand. This
- // cannot be converted to poison as the type cannot be reconstructed.
- // TODO: find a way to support this case.
- if (isMetadataKillLocation(dbgIntr))
- return emitUnsupportedWarning();
- // Drop debug intrinsics if the associated variable information cannot be
- // translated due to cyclic debug metadata.
- // TODO: Support cyclic debug metadata.
- DILocalVariableAttr localVariableAttr =
- matchLocalVariableAttr(dbgIntr->getArgOperand(1));
- if (!localVariableAttr)
- return emitUnsupportedWarning();
- FailureOr<Value> argOperand = convertMetadataValue(dbgIntr->getArgOperand(0));
- if (failed(argOperand))
- return emitError(loc) << "failed to convert a debug intrinsic operand: "
- << diag(*dbgIntr);
-
- // Ensure that the debug intrinsic is inserted right after its operand is
- // defined. Otherwise, the operand might not necessarily dominate the
- // intrinsic. If the defining operation is a terminator, insert the intrinsic
- // into a dominated block.
- OpBuilder::InsertionGuard guard(builder);
- if (Operation *op = argOperand->getDefiningOp();
+/// Ensure that the debug intrinsic is inserted right after its operand is
+/// defined. Otherwise, the operand might not necessarily dominate the
+/// intrinsic. If the defining operation is a terminator, insert the intrinsic
+/// into a dominated block.
+static LogicalResult setDebugIntrinsicBuilderInsertionPoint(
+ mlir::OpBuilder &builder, DominanceInfo &domInfo, Value argOperand) {
+ if (Operation *op = argOperand.getDefiningOp();
op && op->hasTrait<OpTrait::IsTerminator>()) {
// Find a dominated block that can hold the debug intrinsic.
auto dominatedBlocks = domInfo.getNode(op->getBlock())->children();
// If no block is dominated by the terminator, this intrinisc cannot be
// converted.
if (dominatedBlocks.empty())
- return emitUnsupportedWarning();
+ return failure();
// Set insertion point before the terminator, to avoid inserting something
// before landingpads.
Block *dominatedBlock = (*dominatedBlocks.begin())->getBlock();
builder.setInsertionPoint(dominatedBlock->getTerminator());
} else {
- Value insertPt = *argOperand;
- if (auto blockArg = dyn_cast<BlockArgument>(*argOperand)) {
+ Value insertPt = argOperand;
+ if (auto blockArg = dyn_cast<BlockArgument>(argOperand)) {
// The value might be coming from a phi node and is now a block argument,
// which means the insertion point is set to the start of the block. If
// this block is a target destination of an invoke, the insertion point
// must happen after the landing pad operation.
- Block *insertionBlock = argOperand->getParentBlock();
+ Block *insertionBlock = argOperand.getParentBlock();
if (!insertionBlock->empty() &&
isa<LandingpadOp>(insertionBlock->front()))
insertPt = cast<LandingpadOp>(insertionBlock->front()).getRes();
@@ -3084,23 +3077,151 @@ ModuleImport::processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr,
builder.setInsertionPointAfterValue(insertPt);
}
- auto locationExprAttr =
- debugImporter->translateExpression(dbgIntr->getExpression());
+ return success();
+}
+
+void ModuleImport::processDebugOpArgumentsAndInsertionPt(
+ Location loc, DILocalVariableAttr &localVarAttr,
+ DIExpressionAttr &localExprAttr, Value &locVal, bool hasArgList,
+ bool isKillLocation,
+ llvm::function_ref<FailureOr<Value>()> convertArgOperandToValue,
+ llvm::Value *address,
+ llvm::PointerUnion<llvm::Value *, llvm::DILocalVariable *> variable,
+ llvm::DIExpression *expression, DominanceInfo &domInfo) {
+ // Drop debug intrinsics with arg lists.
+ // TODO: Support debug intrinsics that have arg lists.
+ if (hasArgList)
+ return;
+ // Kill locations can have metadata nodes as location operand. This
+ // cannot be converted to poison as the type cannot be reconstructed.
+ // TODO: find a way to support this case.
+ if (isMetadataKillLocation(isKillLocation, address))
+ return;
+ // Drop debug intrinsics if the associated variable information cannot be
+ // translated due to cyclic debug metadata.
+ // TODO: Support cyclic debug metadata.
+ localVarAttr = matchLocalVariableAttr(variable);
+ if (!localVarAttr)
+ return;
+ FailureOr<Value> argOperand = convertArgOperandToValue();
+ if (failed(argOperand)) {
+ emitError(loc) << "failed to convert a debug operand: " << diag(*address);
+ return;
+ }
+
+ if (setDebugIntrinsicBuilderInsertionPoint(builder, domInfo, *argOperand)
+ .failed())
+ return;
+
+ localExprAttr = debugImporter->translateExpression(expression);
+ locVal = *argOperand;
+}
+
+LogicalResult
+ModuleImport::processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr,
+ DominanceInfo &domInfo) {
+ Location loc = translateLoc(dbgIntr->getDebugLoc());
+ auto emitUnsupportedWarning = [&]() {
+ if (emitExpensiveWarnings)
+ emitWarning(loc) << "dropped intrinsic: " << diag(*dbgIntr);
+ return success();
+ };
+
+ DILocalVariableAttr localVariableAttr;
+ DIExpressionAttr locationExprAttr;
+ Value locVal;
+ OpBuilder::InsertionGuard guard(builder);
+ auto convertArgOperandToValue = [&]() {
+ return convertMetadataValue(dbgIntr->getArgOperand(0));
+ };
+
+ processDebugOpArgumentsAndInsertionPt(
+ loc, localVariableAttr, locationExprAttr, locVal, dbgIntr->hasArgList(),
+ dbgIntr->isKillLocation(), convertArgOperandToValue,
+ dbgIntr->getArgOperand(0), dbgIntr->getArgOperand(1),
+ dbgIntr->getExpression(), domInfo);
+
+ if (!localVariableAttr)
+ return emitUnsupportedWarning();
+
+ if (!locVal) // Expected if localVariableAttr is present.
+ return failure();
+
Operation *op =
llvm::TypeSwitch<llvm::DbgVariableIntrinsic *, Operation *>(dbgIntr)
.Case([&](llvm::DbgDeclareInst *) {
return LLVM::DbgDeclareOp::create(
- builder, loc, *argOperand, localVariableAttr, locationExprAttr);
+ builder, loc, locVal, localVariableAttr, locationExprAttr);
})
.Case([&](llvm::DbgValueInst *) {
return LLVM::DbgValueOp::create(
- builder, loc, *argOperand, localVariableAttr, locationExprAttr);
+ builder, loc, locVal, localVariableAttr, locationExprAttr);
});
mapNoResultOp(dbgIntr, op);
setNonDebugMetadataAttrs(dbgIntr, op);
return success();
}
+LogicalResult ModuleImport::processDebugRecord(llvm::DbgRecord &dr,
+ DominanceInfo &domInfo) {
+ Location loc = translateLoc(dr.getDebugLoc());
+ auto emitUnsupportedWarning = [&]() {
+ if (!emitExpensiveWarnings)
+ return success();
+ std::string options;
+ llvm::raw_string_ostream optionsStream(options);
+ dr.print(optionsStream);
+ emitWarning(loc) << "unhandled debug record " << optionsStream.str();
+ return success();
+ };
+
+ OpBuilder::InsertionGuard guard(builder);
+ if (auto *dbgVar = dyn_cast<llvm::DbgVariableRecord>(&dr)) {
+ DILocalVariableAttr localVariableAttr;
+ DIExpressionAttr locationExprAttr;
+ Value locVal;
+ auto convertArgOperandToValue = [&]() -> FailureOr<Value> {
+ llvm::Value *value = dbgVar->getAddress();
+
+ // Return the mapped value if it has been converted before.
+ auto it = valueMapping.find(value);
+ if (it != valueMapping.end())
+ return it->getSecond();
+
+ // Convert constants such as immediate values that have no mapping yet.
+ if (auto *constant = dyn_cast<llvm::Constant>(value))
+ return convertConstantExpr(constant);
+ return failure();
+ };
+
+ processDebugOpArgumentsAndInsertionPt(
+ loc, localVariableAttr, locationExprAttr, locVal, dbgVar->hasArgList(),
+ dbgVar->isKillLocation(), convertArgOperandToValue,
+ dbgVar->getAddress(), dbgVar->getVariable(), dbgVar->getExpression(),
+ domInfo);
+
+ if (!localVariableAttr)
+ return emitUnsupportedWarning();
+
+ if (!locVal) // Expected if localVariableAttr is present.
+ return failure();
+
+ if (dbgVar->isDbgDeclare())
+ LLVM::DbgDeclareOp::create(builder, loc, locVal, localVariableAttr,
+ locationExprAttr);
+ else if (dbgVar->isDbgValue())
+ LLVM::DbgValueOp::create(builder, loc, locVal, localVariableAttr,
+ locationExprAttr);
+ else // isDbgAssign
+ return emitUnsupportedWarning();
+
+ // FIXME: Nothing to map given the source is an LLVM attribute?
+ return success();
+ }
+
+ return emitUnsupportedWarning();
+}
+
LogicalResult ModuleImport::processDebugIntrinsics() {
DominanceInfo domInfo;
for (llvm::Instruction *inst : debugIntrinsics) {
@@ -3111,6 +3232,16 @@ LogicalResult ModuleImport::processDebugIntrinsics() {
return success();
}
+LogicalResult ModuleImport::processDebugRecords() {
+ DominanceInfo domInfo;
+ for (llvm::DbgRecord *dr : debugRecords) {
+ if (failed(processDebugRecord(*dr, domInfo)))
+ return failure();
+ }
+ debugRecords.clear();
+ return success();
+}
+
LogicalResult ModuleImport::processBasicBlock(llvm::BasicBlock *bb,
Block *block) {
builder.setInsertionPointToStart(block);
diff --git a/mlir/test/Target/LLVMIR/Import/debug-info-records.ll b/mlir/test/Target/LLVMIR/Import/debug-info-records.ll
new file mode 100644
index 0000000000000..142356dc0a1e2
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/Import/debug-info-records.ll
@@ -0,0 +1,94 @@
+; RUN: mlir-translate -import-llvm -mlir-print-debuginfo -convert-debug-rec-to-intrinsics -emit-expensive-warnings -split-input-file %s 2>&1 | FileCheck %s
+; RUN: mlir-translate -import-llvm -mlir-print-debuginfo -emit-expensive-warnings -split-input-file %s 2>&1 | FileCheck %s
+
+; CHECK: @callee()
+define void @callee() {
+ ret void
+}
+
+define void @func_with_empty_named_info() {
+ call void @callee()
+ ret void
+}
+
+define void @func_no_debug() {
+ ret void
+}
+
+; CHECK: llvm.func @func_with_debug(%[[ARG0:.*]]: i64
+define void @func_with_debug(i64 %0) !dbg !3 {
+
+ ; CHECK: llvm.intr.dbg.value #di_local_variable = %[[ARG0]] : i64
+ ; CHECK: llvm.intr.dbg.value #di_local_variable1 #llvm.di_expression<[DW_OP_LLVM_fragment(0, 1)]> = %[[ARG0]] : i64
+ ; CHECK: %[[CST:.*]] = llvm.mlir.constant(1 : i32) : i32
+ ; CHECK: %[[ADDR:.*]] = llvm.alloca %[[CST]] x i64
+ ; CHECK: llvm.intr.dbg.declare #di_local_variable2 #llvm.di_expression<[DW_OP_deref, DW_OP_LLVM_convert(4, DW_ATE_signed)]> = %[[ADDR]] : !llvm.ptr
+ %2 = alloca i64, align 8, !dbg !19
+ #dbg_value(i64 %0, !20, !DIExpression(DW_OP_LLVM_fragment, 0, 1), !22)
+ #dbg_declare(ptr %2, !23, !DIExpression(DW_OP_deref, DW_OP_LLVM_convert, 4, DW_ATE_signed), !25)
+ #dbg_value(i64 %0, !26, !DIExpression(), !27)
+ call void @func_no_debug(), !dbg !28
+ call void @func_no_debug(), !db...
[truncated]
|
gysit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that is very nice!
LGTM modulo some nits / details.
gysit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, Thanks!
Effectively means we don't need to call into
llvmModule->convertFromNewDbgValues()anymore. Added a flag to allow users to access the old behavior.