From 3589885d82b6a835a783304111400ec3bb167a6b Mon Sep 17 00:00:00 2001 From: Tobias Gysi Date: Tue, 10 Jan 2023 17:13:33 +0100 Subject: [PATCH] [mlir][llvm] Improve error messages during LLVM IR import. Use the module location instead of unknown location if the imported LLVM IR module does not have more precise debug information. Additionally, use the diagMD function to print the metadata instead of just the metadata kind if the import fails. Reviewed By: definelicht Differential Revision: https://reviews.llvm.org/D141357 --- mlir/lib/Target/LLVMIR/DebugImporter.cpp | 2 +- mlir/lib/Target/LLVMIR/DebugImporter.h | 4 +- mlir/lib/Target/LLVMIR/ModuleImport.cpp | 35 ++++++------ mlir/test/Target/LLVMIR/Import/basic.ll | 10 ++-- mlir/test/Target/LLVMIR/Import/debug-info.ll | 12 ++-- mlir/test/Target/LLVMIR/Import/import-failure.ll | 71 +++++++++++++++++------- 6 files changed, 84 insertions(+), 50 deletions(-) diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp index 5ffcb12..dac737d 100644 --- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp +++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp @@ -191,7 +191,7 @@ DINodeAttr DebugImporter::translate(llvm::DINode *node) { Location DebugImporter::translateLoc(llvm::DILocation *loc) { if (!loc) - return UnknownLoc::get(context); + return mlirModule.getLoc(); // Get the file location of the instruction. Location result = FileLineColLoc::get(context, loc->getFilename(), diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.h b/mlir/lib/Target/LLVMIR/DebugImporter.h index 4be4aa1..e50f3b9 100644 --- a/mlir/lib/Target/LLVMIR/DebugImporter.h +++ b/mlir/lib/Target/LLVMIR/DebugImporter.h @@ -29,7 +29,8 @@ namespace detail { class DebugImporter { public: - DebugImporter(MLIRContext *context) : context(context) {} + DebugImporter(ModuleOp mlirModule) + : context(mlirModule.getContext()), mlirModule(mlirModule) {} /// Translates the given LLVM debug location to an MLIR location. Location translateLoc(llvm::DILocation *loc); @@ -69,6 +70,7 @@ private: DenseMap nodeToAttr; MLIRContext *context; + ModuleOp mlirModule; }; } // namespace detail diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp index 03287e9..b85fae1 100644 --- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp +++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp @@ -354,7 +354,7 @@ ModuleImport::ModuleImport(ModuleOp mlirModule, mlirModule(mlirModule), llvmModule(std::move(llvmModule)), iface(mlirModule->getContext()), typeTranslator(*mlirModule->getContext()), - debugImporter(std::make_unique(mlirModule->getContext())) { + debugImporter(std::make_unique(mlirModule)) { builder.setInsertionPointToStart(mlirModule.getBody()); } @@ -636,13 +636,13 @@ LogicalResult ModuleImport::convertGlobals() { globalVar.getName() == getGlobalDtorsVarName()) { if (failed(convertGlobalCtorsAndDtors(&globalVar))) { return emitError(mlirModule.getLoc()) - << "unhandled global variable " << diag(globalVar); + << "unhandled global variable: " << diag(globalVar); } continue; } if (failed(convertGlobal(&globalVar))) { return emitError(mlirModule.getLoc()) - << "unhandled global variable " << diag(globalVar); + << "unhandled global variable: " << diag(globalVar); } } return success(); @@ -664,7 +664,9 @@ void ModuleImport::setNonDebugMetadataAttrs(llvm::Instruction *inst, continue; if (failed(iface.setMetadataAttrs(builder, kind, node, op, *this))) { Location loc = debugImporter->translateLoc(inst->getDebugLoc()); - emitWarning(loc) << "unhandled metadata (" << kind << ") " << diag(*inst); + emitWarning(loc) << "unhandled metadata: " + << diagMD(node, llvmModule.get()) << " on " + << diag(*inst); } } } @@ -967,8 +969,7 @@ ModuleImport::getConstantsToConvert(llvm::Constant *constant) { } FailureOr ModuleImport::convertConstant(llvm::Constant *constant) { - // Constants have no location attached. - Location loc = UnknownLoc::get(context); + Location loc = mlirModule.getLoc(); // Convert constants that can be represented as attributes. if (Attribute attr = getConstantAsAttr(constant)) { @@ -1062,7 +1063,7 @@ FailureOr ModuleImport::convertConstant(llvm::Constant *constant) { return root; } - return emitError(loc) << "unhandled constant " << diag(*constant); + return emitError(loc) << "unhandled constant: " << diag(*constant); } FailureOr ModuleImport::convertConstantExpr(llvm::Constant *constant) { @@ -1107,10 +1108,10 @@ FailureOr ModuleImport::convertValue(llvm::Value *value) { if (auto *constant = dyn_cast(value)) return convertConstantExpr(constant); - Location loc = UnknownLoc::get(context); + Location loc = mlirModule.getLoc(); if (auto *inst = dyn_cast(value)) loc = translateLoc(inst->getDebugLoc()); - return emitError(loc) << "unhandled value " << diag(*value); + return emitError(loc) << "unhandled value: " << diag(*value); } FailureOr> @@ -1187,7 +1188,7 @@ LogicalResult ModuleImport::convertIntrinsic(llvm::CallInst *inst) { return success(); Location loc = translateLoc(inst->getDebugLoc()); - return emitError(loc) << "unhandled intrinsic " << diag(*inst); + return emitError(loc) << "unhandled intrinsic: " << diag(*inst); } LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) { @@ -1364,7 +1365,7 @@ LogicalResult ModuleImport::convertInstruction(llvm::Instruction *inst) { if (succeeded(convertInstructionImpl(builder, inst, *this))) return success(); - return emitError(loc) << "unhandled instruction " << diag(*inst); + return emitError(loc) << "unhandled instruction: " << diag(*inst); } LogicalResult ModuleImport::processInstruction(llvm::Instruction *inst) { @@ -1432,7 +1433,7 @@ LogicalResult ModuleImport::processFunction(llvm::Function *func) { builder.setInsertionPoint(mlirModule.getBody(), mlirModule.getBody()->end()); LLVMFuncOp funcOp = builder.create( - UnknownLoc::get(context), func->getName(), functionType, + mlirModule.getLoc(), func->getName(), functionType, convertLinkageFromLLVM(func->getLinkage()), dsoLocal, cconv); // Set the function debug information if available. @@ -1471,8 +1472,7 @@ LogicalResult ModuleImport::processFunction(llvm::Function *func) { if (FlatSymbolRefAttr personality = getPersonalityAsAttr(func)) funcOp.setPersonalityAttr(personality); else if (func->hasPersonalityFn()) - emitWarning(UnknownLoc::get(context), - "could not deduce personality, skipping it"); + emitWarning(funcOp.getLoc(), "could not deduce personality, skipping it"); if (func->hasGC()) funcOp.setGarbageCollector(StringRef(func->getGC())); @@ -1487,8 +1487,9 @@ LogicalResult ModuleImport::processFunction(llvm::Function *func) { if (!iface.isConvertibleMetadata(kind)) continue; if (failed(iface.setMetadataAttrs(builder, kind, node, funcOp, *this))) { - emitWarning(funcOp->getLoc()) - << "unhandled function metadata (" << kind << ") " << diag(*func); + emitWarning(funcOp.getLoc()) + << "unhandled function metadata: " << diagMD(node, llvmModule.get()) + << " on " << diag(*func); } } @@ -1536,7 +1537,7 @@ LogicalResult ModuleImport::processBasicBlock(llvm::BasicBlock *bb, setNonDebugMetadataAttrs(&inst, op); } else if (inst.getOpcode() != llvm::Instruction::PHI) { Location loc = debugImporter->translateLoc(inst.getDebugLoc()); - emitWarning(loc) << "dropped instruction " << diag(inst); + emitWarning(loc) << "dropped instruction: " << diag(inst); } } return success(); diff --git a/mlir/test/Target/LLVMIR/Import/basic.ll b/mlir/test/Target/LLVMIR/Import/basic.ll index 1bbe370..dcc72c1 100644 --- a/mlir/test/Target/LLVMIR/Import/basic.ll +++ b/mlir/test/Target/LLVMIR/Import/basic.ll @@ -1,7 +1,7 @@ ; RUN: mlir-translate -import-llvm %s | FileCheck %s ; RUN: mlir-translate -import-llvm -mlir-print-debuginfo %s | FileCheck %s --check-prefix=CHECK-DBG -; CHECK-DBG: #[[UNKNOWNLOC:.+]] = loc(unknown) +; CHECK-DBG: #[[MODULELOC:.+]] = loc({{.*}}basic.ll{{.*}}:0:0) @global = external global double, align 8 @@ -10,7 +10,7 @@ declare float @fe(i32) ; FIXME: function attributes. ; CHECK-LABEL: llvm.func internal @f1(%arg0: i64) -> i32 attributes {dso_local} { -; CHECK-DBG: llvm.func internal @f1(%arg0: i64 loc(unknown)) -> i32 attributes {dso_local} { +; CHECK-DBG: llvm.func internal @f1(%arg0: i64 loc({{.*}}basic.ll{{.*}}:0:0)) -> i32 attributes {dso_local} { ; CHECK: %[[c2:[0-9]+]] = llvm.mlir.constant(2 : i32) : i32 ; CHECK: %[[c1:[0-9]+]] = llvm.mlir.constant(true) : i1 ; CHECK: %[[c43:[0-9]+]] = llvm.mlir.constant(43 : i32) : i32 @@ -19,7 +19,7 @@ define internal dso_local i32 @f1(i64 %a) norecurse { entry: ; CHECK: %{{[0-9]+}} = llvm.inttoptr %arg0 : i64 to !llvm.ptr %aa = inttoptr i64 %a to i64* -; CHECK-DBG: llvm.mlir.addressof @global : !llvm.ptr loc(#[[UNKNOWNLOC]]) +; CHECK-DBG: llvm.mlir.addressof @global : !llvm.ptr loc(#[[MODULELOC]]) ; %[[addrof:[0-9]+]] = llvm.mlir.addressof @global : !llvm.ptr ; %[[addrof2:[0-9]+]] = llvm.mlir.addressof @global : !llvm.ptr ; %{{[0-9]+}} = llvm.inttoptr %arg0 : i64 to !llvm.ptr @@ -28,7 +28,7 @@ entry: %bb = ptrtoint double* @global to i64 %cc = getelementptr double, double* @global, i32 3 ; CHECK: %[[b:[0-9]+]] = llvm.trunc %arg0 : i64 to i32 -; CHECK-DBG: llvm.trunc %arg0 : i64 to i32 loc(#[[UNKNOWNLOC]]) +; CHECK-DBG: llvm.trunc %arg0 : i64 to i32 loc(#[[MODULELOC]]) %b = trunc i64 %a to i32 ; CHECK: %[[c:[0-9]+]] = llvm.call @fe(%[[b]]) : (i32) -> f32 %c = call float @fe(i32 %b) @@ -52,7 +52,7 @@ if.end: ; CHECK: llvm.return %[[c43]] ret i32 43 } -; CHECK-DBG: } loc(#[[UNKNOWNLOC]]) +; CHECK-DBG: } loc(#[[MODULELOC]]) @_ZTIi = external dso_local constant i8* diff --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll index f7513d5..6e39f37 100644 --- a/mlir/test/Target/LLVMIR/Import/debug-info.ll +++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll @@ -1,21 +1,21 @@ ; RUN: mlir-translate -import-llvm -mlir-print-debuginfo -split-input-file %s | FileCheck %s -; CHECK-LABEL: @unknown( -define i32 @unknown(i32 %0) { +; CHECK: #[[$MODULELOC:.+]] = loc({{.*}}debug-info.ll{{.*}}:0:0) + +; CHECK-LABEL: @module_loc( +define i32 @module_loc(i32 %0) { entry: br label %next end: - ; CHECK: ^{{.*}}(%{{.+}}: i32 loc(unknown)): + ; CHECK: ^{{.*}}(%{{.+}}: i32 loc({{.*}}debug-info.ll{{.*}}:0:0)): %1 = phi i32 [ %2, %next ] ret i32 %1 next: - ; CHECK: = llvm.mul %{{.+}}, %{{.+}} : i32 loc(#[[UNKNOWNLOC:.+]]) + ; CHECK: = llvm.mul %{{.+}}, %{{.+}} : i32 loc(#[[$MODULELOC]]) %2 = mul i32 %0, %0 br label %end } -; CHECK: #[[UNKNOWNLOC:.+]] = loc(unknown) - ; // ----- ; CHECK-LABEL: @instruction_loc diff --git a/mlir/test/Target/LLVMIR/Import/import-failure.ll b/mlir/test/Target/LLVMIR/Import/import-failure.ll index 99a35af..d598175 100644 --- a/mlir/test/Target/LLVMIR/Import/import-failure.ll +++ b/mlir/test/Target/LLVMIR/Import/import-failure.ll @@ -1,6 +1,7 @@ ; RUN: not mlir-translate -import-llvm -split-input-file %s 2>&1 | FileCheck %s -; CHECK: error: unhandled instruction indirectbr i8* %dst, [label %bb1, label %bb2] +; CHECK: import-failure.ll +; CHECK-SAME: error: unhandled instruction: indirectbr i8* %dst, [label %bb1, label %bb2] define i32 @unhandled_instruction(i8* %dst) { indirectbr i8* %dst, [label %bb1, label %bb2] bb1: @@ -11,7 +12,8 @@ bb2: ; // ----- -; CHECK: unhandled value ptr asm "bswap $0", "=r,r" +; CHECK: import-failure.ll +; CHECK-SAME: error: unhandled value: ptr asm "bswap $0", "=r,r" define i32 @unhandled_value(i32 %arg0) { %1 = call i32 asm "bswap $0", "=r,r"(i32 %arg0) ret i32 %1 @@ -19,7 +21,10 @@ define i32 @unhandled_value(i32 %arg0) { ; // ----- -; CHECK: error: unhandled constant i8* blockaddress(@unhandled_constant, %bb1) +; CHECK: import-failure.ll +; CHECK-SAME: error: unhandled constant: i8* blockaddress(@unhandled_constant, %bb1) +; CHECK: import-failure.ll +; CHECK-SAME: error: unhandled instruction: ret i8* blockaddress(@unhandled_constant, %bb1) define i8* @unhandled_constant() { bb1: ret i8* blockaddress(@unhandled_constant, %bb1) @@ -27,8 +32,10 @@ bb1: ; // ----- -; CHECK: error: unhandled constant i8* blockaddress(@unhandled_global, %bb1) -; CHECK: error: unhandled global variable @private = private global i8* blockaddress(@unhandled_global, %bb1) +; CHECK: import-failure.ll +; CHECK-SAME: error: unhandled constant: i8* blockaddress(@unhandled_global, %bb1) +; CHECK: import-failure.ll +; CHECK-SAME: error: unhandled global variable: @private = private global i8* blockaddress(@unhandled_global, %bb1) @private = private global i8* blockaddress(@unhandled_global, %bb1) define void @unhandled_global() { @@ -40,7 +47,8 @@ bb1: declare void @llvm.gcroot(ptr %arg0, ptr %arg1) -; CHECK: error: unhandled intrinsic call void @llvm.gcroot(ptr %arg0, ptr %arg1) +; CHECK: import-failure.ll +; CHECK-SAME: error: unhandled intrinsic: call void @llvm.gcroot(ptr %arg0, ptr %arg1) define void @unhandled_intrinsic(ptr %arg0, ptr %arg1) { call void @llvm.gcroot(ptr %arg0, ptr %arg1) ret void @@ -48,8 +56,9 @@ define void @unhandled_intrinsic(ptr %arg0, ptr %arg1) { ; // ----- -; CHECK: warning: unhandled metadata (2) br i1 %arg1, label %bb1, label %bb2, !prof !0 -define i64 @cond_br(i1 %arg1, i64 %arg2) { +; CHECK: import-failure.ll +; CHECK-SAME: warning: unhandled metadata: !0 = !{!"unknown metadata"} on br i1 %arg1, label %bb1, label %bb2, !prof !0 +define i64 @unhandled_metadata(i1 %arg1, i64 %arg2) { entry: br i1 %arg1, label %bb1, label %bb2, !prof !0 bb1: @@ -62,9 +71,20 @@ bb2: ; // ----- +; CHECK: import-failure.ll +; CHECK-SAME: warning: unhandled function metadata: !0 = !{!"unknown metadata"} on define void @unhandled_func_metadata(i1 %arg1, i64 %arg2) !prof !0 +define void @unhandled_func_metadata(i1 %arg1, i64 %arg2) !prof !0 { + ret void +} + +!0 = !{!"unknown metadata"} + +; // ----- + declare void @llvm.dbg.value(metadata, metadata, metadata) -; CHECK: 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: 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 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 ret void @@ -82,7 +102,8 @@ define void @dropped_instruction(i64 %arg1) { ; // ----- ; global_ctors requires the appending linkage type. -; CHECK: error: unhandled global variable @llvm.global_ctors +; CHECK: import-failure.ll +; CHECK-SAME: error: unhandled global variable: @llvm.global_ctors @llvm.global_ctors = global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 0, ptr @foo, ptr null }] define void @foo() { @@ -92,7 +113,8 @@ define void @foo() { ; // ----- ; global_dtors with non-null data fields cannot be represented in MLIR. -; CHECK: error: unhandled global variable @llvm.global_dtors +; CHECK: import-failure.ll +; CHECK-SAME: error: unhandled global variable: @llvm.global_dtors @llvm.global_dtors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 0, ptr @foo, ptr @foo }] define void @foo() { @@ -102,7 +124,8 @@ define void @foo() { ; // ----- ; global_ctors without a data field should not be imported. -; CHECK: error: unhandled global variable @llvm.global_ctors +; CHECK: import-failure.ll +; CHECK-SAME: error: unhandled global variable: @llvm.global_ctors @llvm.global_ctors = appending global [1 x { i32, ptr }] [{ i32, ptr } { i32 0, ptr @foo }] define void @foo() { @@ -112,7 +135,8 @@ define void @foo() { ; // ----- ; global_dtors with a wrong argument order should not be imported. -; CHECK: error: unhandled global variable @llvm.global_dtors +; CHECK: import-failure.ll +; CHECK-SAME: error: unhandled global variable: @llvm.global_dtors @llvm.global_dtors = appending global [1 x { ptr, i32, ptr }] [{ ptr, i32, ptr } { ptr @foo, i32 0, ptr null }] define void @foo() { @@ -121,7 +145,8 @@ define void @foo() { ; // ----- -; CHECK: error: TBAA root node must have non-empty identity: !2 = !{!""} +; CHECK: import-failure.ll +; CHECK-SAME: error: TBAA root node must have non-empty identity: !2 = !{!""} define dso_local void @tbaa(ptr %0) { store i8 1, ptr %0, align 4, !tbaa !2 ret void @@ -133,7 +158,8 @@ define dso_local void @tbaa(ptr %0) { ; // ----- -; CHECK: error: unsupported TBAA node format: !0 = !{!1, i64 0, i64 0} +; CHECK: import-failure.ll +; CHECK-SAME: error: unsupported TBAA node format: !0 = !{!1, i64 0, i64 0} define dso_local void @tbaa(ptr %0) { store i8 1, ptr %0, align 4, !tbaa !2 ret void @@ -145,7 +171,8 @@ define dso_local void @tbaa(ptr %0) { ; // ----- -; CHECK: error: operand '1' must be MDNode: !1 = !{!"omnipotent char", i64 0, i64 0} +; CHECK: import-failure.ll +; CHECK-SAME: error: operand '1' must be MDNode: !1 = !{!"omnipotent char", i64 0, i64 0} define dso_local void @tbaa(ptr %0) { store i8 1, ptr %0, align 4, !tbaa !2 ret void @@ -157,7 +184,8 @@ define dso_local void @tbaa(ptr %0) { ; // ----- -; CHECK: error: missing member offset: !1 = !{!"agg_t", !2, i64 0, !2} +; CHECK: import-failure.ll +; CHECK-SAME: error: missing member offset: !1 = !{!"agg_t", !2, i64 0, !2} define dso_local void @tbaa(ptr %0) { store i8 1, ptr %0, align 4, !tbaa !3 ret void @@ -170,7 +198,8 @@ define dso_local void @tbaa(ptr %0) { ; // ----- -; CHECK: error: operand '4' must be ConstantInt: !1 = !{!"agg_t", !2, i64 0, !2, double 1.000000e+00} +; CHECK: import-failure.ll +; CHECK-SAME: error: operand '4' must be ConstantInt: !1 = !{!"agg_t", !2, i64 0, !2, double 1.000000e+00} define dso_local void @tbaa(ptr %0) { store i8 1, ptr %0, align 4, !tbaa !3 ret void @@ -183,7 +212,8 @@ define dso_local void @tbaa(ptr %0) { ; // ----- -; CHECK: error: operand '3' must be ConstantInt: !0 = !{!1, !1, i64 0, double 1.000000e+00} +; CHECK: import-failure.ll +; CHECK-SAME: error: operand '3' must be ConstantInt: !0 = !{!1, !1, i64 0, double 1.000000e+00} define dso_local void @tbaa(ptr %0) { store i8 1, ptr %0, align 4, !tbaa !2 ret void @@ -195,7 +225,8 @@ define dso_local void @tbaa(ptr %0) { ; // ----- -; CHECK: error: unsupported TBAA node format: !0 = !{!1, !1, i64 0, i64 4} +; CHECK: import-failure.ll +; CHECK-SAME: error: unsupported TBAA node format: !0 = !{!1, !1, i64 0, i64 4} define dso_local void @tbaa(ptr %0) { store i32 1, ptr %0, align 4, !tbaa !2 ret void -- 2.7.4