[Mlir-commits] [mlir] b808648 - [mlir][LLVM] Delay debug intrinsic import to ensure dominance
Christian Ulmann
llvmlistbot at llvm.org
Wed Aug 9 06:07:56 PDT 2023
Author: Christian Ulmann
Date: 2023-08-09T13:03:45Z
New Revision: b808648ac1308cc2563fdc1fc7796da704dc1ce5
URL: https://github.com/llvm/llvm-project/commit/b808648ac1308cc2563fdc1fc7796da704dc1ce5
DIFF: https://github.com/llvm/llvm-project/commit/b808648ac1308cc2563fdc1fc7796da704dc1ce5.diff
LOG: [mlir][LLVM] Delay debug intrinsic import to ensure dominance
This commit ensures that debug intrinsics are imported after all other
instructions of a function were imported.
Debug intrinsics in LLVM are excluded from the define-before-use
invariant, i.e., they can reference SSA values that do not dominate
them. So far, we had implemented checks to stop the import of such
intrinsics, but there were always additional cases that were not
covered (the latest being terminators that define such used values).
Reviewed By: gysit, zero9178
Differential Revision: https://reviews.llvm.org/D157496
Added:
Modified:
mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
mlir/include/mlir/Target/LLVMIR/ModuleImport.h
mlir/lib/Target/LLVMIR/ModuleImport.cpp
mlir/test/Target/LLVMIR/Import/debug-info.ll
mlir/test/Target/LLVMIR/Import/import-failure.ll
Removed:
################################################################################
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
index 88316bedcce8fc..291b2290961689 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
@@ -542,33 +542,10 @@ class LLVM_DbgIntrOp<string name, string argName, list<Trait> traits = []>
});
}];
let mlirBuilder = [{
- // Drop debug intrinsics with a non-empty debug expression.
- // TODO: Support debug intrinsics that evaluate a debug expression.
- auto *dbgIntr = cast<llvm::DbgVariableIntrinsic>(inst);
- if (dbgIntr->hasArgList() || dbgIntr->getExpression()->getNumElements() != 0)
- return success();
- // Convert the value/address operand late since it cannot be a debug
- // metadata argument list at this stage. Generating the conversion using an
- // argument variable would not work here, since the builder variables are
- // converted before entering the builder, which would result in an error
- // when attempting to convert an argument list.
-
- FailureOr<Value> argOperand = moduleImport.convertMetadataValue(llvmOperands[0]);
- // Drop the intrinsic when its operand could not be converted. This can
- // happen for use before definition cases that are allowed for debug
- // intrinsics.
- // TODO: Implement a two pass solution that translates the debug intrinsics
- // after the entire function as been translated.
- if (failed(argOperand))
- return success();
-
- // Ensure that the debug instrinsic is inserted right after its operand is
- // defined. Otherwise, the operand might not necessarily dominate the
- // intrinsic.
- OpBuilder::InsertionGuard guard($_builder);
- $_builder.setInsertionPointAfterValue(*argOperand);
- $_op = $_builder.create<$_qualCppClassName>($_location,
- *argOperand, $_var_attr($varInfo));
+ // Add debug intrindic to the list of intrinsics that need to be converted once the
+ // full function was converted.
+ moduleImport.addDebugIntrinsic(inst);
+ return success();
}];
let assemblyFormat = [{
qualified($varInfo) `=` $}] # argName #
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index 8b7bf10333cf23..9bedc84e0bfa16 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -23,6 +23,7 @@
namespace llvm {
class BasicBlock;
class CallBase;
+class DbgVariableIntrinsic;
class Function;
class Instruction;
class Value;
@@ -205,12 +206,17 @@ class ModuleImport {
FailureOr<SmallVector<AliasScopeAttr>>
lookupAliasScopeAttrs(const llvm::MDNode *node) const;
+ /// Adds a debug intrinsics to the list of intrinsics that should be converted
+ /// after the function conversion has finished.
+ void addDebugIntrinsic(llvm::CallInst *intrinsic);
+
private:
- /// Clears the block and value mapping before processing a new region.
- void clearBlockAndValueMapping() {
+ /// Clears the accumulated state before processing a new region.
+ void clearRegionState() {
valueMapping.clear();
noResultOpMapping.clear();
blockMapping.clear();
+ debugIntrinsics.clear();
}
/// Sets the constant insertion point to the start of the given block.
void setConstantInsertionPointToStart(Block *block) {
@@ -227,6 +233,12 @@ class ModuleImport {
FlatSymbolRefAttr getPersonalityAsAttr(llvm::Function *func);
/// Imports `bb` into `block`, which must be initially empty.
LogicalResult processBasicBlock(llvm::BasicBlock *bb, Block *block);
+ /// Converts all debug intrinsics in `debugIntrinsics`. Assumes that the
+ /// function containing the intrinsics has been fully converted to MLIR.
+ LogicalResult processDebugIntrinsics();
+ /// Converts a single debug intrinsic.
+ LogicalResult processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr,
+ DominanceInfo &domInfo);
/// Converts an LLVM intrinsic to an MLIR LLVM dialect operation if an MLIR
/// counterpart exists. Otherwise, returns failure.
LogicalResult convertIntrinsic(llvm::CallInst *inst);
@@ -339,6 +351,9 @@ class ModuleImport {
/// operations for all operations that return no result. All operations that
/// return a result have a valueMapping entry instead.
DenseMap<llvm::Instruction *, Operation *> noResultOpMapping;
+ /// Function-local list of debug intrinsics that need to be imported after the
+ /// function conversion has finished.
+ SetVector<llvm::Instruction *> debugIntrinsics;
/// 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/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 4cfce7119362e2..c71e87ed2673cc 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -29,6 +29,7 @@
#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/TypeSwitch.h"
#include "llvm/IR/Comdat.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/InlineAsm.h"
@@ -482,6 +483,10 @@ ModuleImport::lookupAliasScopeAttrs(const llvm::MDNode *node) const {
return aliasScopes;
}
+void ModuleImport::addDebugIntrinsic(llvm::CallInst *intrinsic) {
+ debugIntrinsics.insert(intrinsic);
+}
+
LogicalResult ModuleImport::convertMetadata() {
OpBuilder::InsertionGuard guard(builder);
builder.setInsertionPointToEnd(mlirModule.getBody());
@@ -838,7 +843,7 @@ LogicalResult ModuleImport::convertGlobal(llvm::GlobalVariable *globalVar) {
globalInsertionOp = globalOp;
if (globalVar->hasInitializer() && !valueAttr) {
- clearBlockAndValueMapping();
+ clearRegionState();
Block *block = builder.createBlock(&globalOp.getInitializerRegion());
setConstantInsertionPointToStart(block);
FailureOr<Value> initializer =
@@ -1490,14 +1495,11 @@ LogicalResult ModuleImport::processInstruction(llvm::Instruction *inst) {
// FIXME: Support uses of SubtargetData.
// FIXME: Add support for call / operand attributes.
// FIXME: Add support for the indirectbr, cleanupret, catchret, catchswitch,
- // callbr, vaarg, landingpad, catchpad, cleanuppad instructions.
+ // callbr, vaarg, catchpad, cleanuppad instructions.
// Convert LLVM intrinsics calls to MLIR intrinsics.
- if (auto *callInst = dyn_cast<llvm::CallInst>(inst)) {
- llvm::Function *callee = callInst->getCalledFunction();
- if (callee && callee->isIntrinsic())
- return convertIntrinsic(callInst);
- }
+ if (auto *intrinsic = dyn_cast<llvm::IntrinsicInst>(inst))
+ return convertIntrinsic(intrinsic);
// Convert all remaining LLVM instructions to MLIR operations.
return convertInstruction(inst);
@@ -1654,7 +1656,7 @@ void ModuleImport::convertParameterAttributes(llvm::Function *func,
}
LogicalResult ModuleImport::processFunction(llvm::Function *func) {
- clearBlockAndValueMapping();
+ clearRegionState();
auto functionType =
dyn_cast<LLVMFunctionType>(convertType(func->getFunctionType()));
@@ -1736,11 +1738,79 @@ LogicalResult ModuleImport::processFunction(llvm::Function *func) {
// value once a block is translated.
SetVector<llvm::BasicBlock *> blocks = getTopologicallySortedBlocks(func);
setConstantInsertionPointToStart(lookupBlock(blocks.front()));
- for (llvm::BasicBlock *bb : blocks) {
+ for (llvm::BasicBlock *bb : blocks)
if (failed(processBasicBlock(bb, lookupBlock(bb))))
return failure();
+
+ // Process the debug intrinsics that require a delayed conversion after
+ // everything else was converted.
+ if (failed(processDebugIntrinsics()))
+ return failure();
+
+ return success();
+}
+
+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 a non-empty debug expression.
+ // TODO: Support debug intrinsics that evaluate a debug expression.
+ if (dbgIntr->hasArgList() || dbgIntr->getExpression()->getNumElements() != 0)
+ return emitUnsupportedWarning();
+ FailureOr<Value> argOperand = convertMetadataValue(dbgIntr->getArgOperand(0));
+ if (failed(argOperand))
+ return failure();
+
+ // Ensure that the debug instrinsic 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();
+ 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();
+ // Set insertion point before the terminator, to avoid inserting something
+ // before landingpads.
+ Block *dominatedBlock = (*dominatedBlocks.begin())->getBlock();
+ builder.setInsertionPoint(dominatedBlock->getTerminator());
+ } else {
+ builder.setInsertionPointAfterValue(*argOperand);
}
+ DILocalVariableAttr localVariableAttr =
+ matchLocalVariableAttr(dbgIntr->getArgOperand(1));
+ Operation *op =
+ llvm::TypeSwitch<llvm::DbgVariableIntrinsic *, Operation *>(dbgIntr)
+ .Case([&](llvm::DbgDeclareInst *) {
+ return builder.create<LLVM::DbgDeclareOp>(loc, *argOperand,
+ localVariableAttr);
+ })
+ .Case([&](llvm::DbgValueInst *) {
+ return builder.create<LLVM::DbgValueOp>(loc, *argOperand,
+ localVariableAttr);
+ });
+ mapNoResultOp(dbgIntr, op);
+ setNonDebugMetadataAttrs(dbgIntr, op);
+ return success();
+}
+LogicalResult ModuleImport::processDebugIntrinsics() {
+ DominanceInfo domInfo;
+ for (llvm::Instruction *inst : debugIntrinsics) {
+ auto *intrCall = cast<llvm::DbgVariableIntrinsic>(inst);
+ if (failed(processDebugIntrinsic(intrCall, domInfo)))
+ return failure();
+ }
return success();
}
@@ -1751,6 +1821,11 @@ LogicalResult ModuleImport::processBasicBlock(llvm::BasicBlock *bb,
if (failed(processInstruction(&inst)))
return failure();
+ // Skip additional processing when the instructions is a debug intrinsics
+ // that was not yet converted.
+ if (debugIntrinsics.contains(&inst))
+ continue;
+
// Set the non-debug metadata attributes on the imported operation and emit
// a warning if an instruction other than a phi instruction is dropped
// during the import.
diff --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll
index 1e2fecbdbe931d..35fbbf310be6d2 100644
--- a/mlir/test/Target/LLVMIR/Import/debug-info.ll
+++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll
@@ -353,6 +353,8 @@ declare !dbg !3 void @variadic_func()
; // -----
define void @dbg_use_before_def(ptr %arg) {
+ ; CHECK: llvm.getelementptr
+ ; CHECK-NEXT: llvm.intr.dbg.value
call void @llvm.dbg.value(metadata ptr %dbg_arg, metadata !7, metadata !DIExpression()), !dbg !9
%dbg_arg = getelementptr double, ptr %arg, i64 16
ret void
@@ -409,6 +411,67 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
; // -----
+declare i64 @callee()
+declare i32 @personality(...)
+
+; CHECK-LABEL: @dbg_broken_dominance_invoke
+define void @dbg_broken_dominance_invoke() personality ptr @personality {
+ %1 = invoke i64 @callee()
+ to label %b1 unwind label %b2
+b1:
+; CHECK: llvm.intr.dbg.value
+ call void @llvm.dbg.value(metadata i64 %1, metadata !7, metadata !DIExpression()), !dbg !9
+ ret void
+b2:
+ %2 = landingpad { ptr, i32 }
+ cleanup
+ ret void
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!1}
+!llvm.module.flags = !{!0}
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+!1 = distinct !DICompileUnit(language: DW_LANG_C, file: !2)
+!2 = !DIFile(filename: "debug-info.ll", directory: "/")
+!7 = !DILocalVariable(scope: !8, name: "var", file: !2);
+!8 = distinct !DISubprogram(name: "dbg_broken_dominance_invoke", scope: !2, file: !2, spFlags: DISPFlagDefinition, unit: !1)
+!9 = !DILocation(line: 1, column: 2, scope: !8)
+
+; // -----
+
+declare i64 @callee()
+declare i32 @personality(...)
+
+; CHECK-LABEL: @dbg_broken_dominance_invoke_reordered
+define void @dbg_broken_dominance_invoke_reordered() personality ptr @personality {
+ %1 = invoke i64 @callee()
+ to label %b2 unwind label %b1
+b1:
+; CHECK: landingpad
+; CHECK: llvm.intr.dbg.value
+ %2 = landingpad { ptr, i32 }
+ cleanup
+ call void @llvm.dbg.value(metadata i64 %1, metadata !7, metadata !DIExpression()), !dbg !9
+ ret void
+b2:
+ ret void
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!1}
+!llvm.module.flags = !{!0}
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+!1 = distinct !DICompileUnit(language: DW_LANG_C, file: !2)
+!2 = !DIFile(filename: "debug-info.ll", directory: "/")
+!7 = !DILocalVariable(scope: !8, name: "var", file: !2);
+!8 = distinct !DISubprogram(name: "dbg_broken_dominance_invoke", scope: !2, file: !2, spFlags: DISPFlagDefinition, unit: !1)
+!9 = !DILocation(line: 1, column: 2, scope: !8)
+
+; // -----
+
; CHECK-DAG: #[[NAMESPACE:.+]] = #llvm.di_namespace<name = "std", exportSymbols = false>
; CHECK-DAG: #[[SUBPROGRAM:.+]] = #llvm.di_subprogram<compileUnit = #{{.*}}, scope = #[[NAMESPACE]], name = "namespace"
diff --git a/mlir/test/Target/LLVMIR/Import/import-failure.ll b/mlir/test/Target/LLVMIR/Import/import-failure.ll
index b41226b75dacc0..a753c464335237 100644
--- a/mlir/test/Target/LLVMIR/Import/import-failure.ll
+++ b/mlir/test/Target/LLVMIR/Import/import-failure.ll
@@ -62,9 +62,9 @@ define void @unhandled_intrinsic() gc "example" {
declare void @llvm.dbg.value(metadata, metadata, metadata)
; CHECK: import-failure.ll
-; CHECK-SAME: warning: dropped instruction: call void @llvm.dbg.value(metadata i64 %arg1, metadata !3, metadata !DIExpression(DW_OP_plus_uconst, 42, DW_OP_stack_value)), !dbg !5
+; CHECK-SAME: warning: dropped intrinsic: call void @llvm.dbg.value(metadata i64 %arg1, metadata !3, metadata !DIExpression(DW_OP_plus_uconst, 42, DW_OP_stack_value)), !dbg !5
; CHECK: import-failure.ll
-; CHECK-SAME: warning: dropped instruction: call void @llvm.dbg.value(metadata !DIArgList(i64 %arg1, i64 undef), metadata !3, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_constu, 1, DW_OP_mul, DW_OP_plus, DW_OP_stack_value)), !dbg !5
+; CHECK-SAME: warning: dropped intrinsic: call void @llvm.dbg.value(metadata !DIArgList(i64 %arg1, i64 undef), metadata !3, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_constu, 1, DW_OP_mul, DW_OP_plus, DW_OP_stack_value)), !dbg !5
define void @dropped_instruction(i64 %arg1) {
call void @llvm.dbg.value(metadata i64 %arg1, metadata !3, metadata !DIExpression(DW_OP_plus_uconst, 42, DW_OP_stack_value)), !dbg !5
call void @llvm.dbg.value(metadata !DIArgList(i64 %arg1, i64 undef), metadata !3, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_constu, 1, DW_OP_mul, DW_OP_plus, DW_OP_stack_value)), !dbg !5
More information about the Mlir-commits
mailing list