[mlir][LLVM] Fix dbg intrinsic import of killed locations

[Dinistro]

Summary

This commit ensures that debug intrinsics of killed variables do not
cause a crash of the importer. Killed locations are usually undef
constants, but in infrequent cases can also be metadata nodes, which
caused problems.

[zero9178]

mlir/lib/Target/LLVMIR/ModuleImport.cpp

1783–1784

I think it'd be more robust to not even attempt convertMetadataValue in this case. Checking dbgInstr->isKillLocation() && !isa<ValueAsMetadata>() should catch this case, right?

[Dinistro]

address review comments, add expensive warning, and move the test.

[Dinistro]

mlir/lib/Target/LLVMIR/ModuleImport.cpp

1783–1784

Good idea. I moved the checkout into a static function, as it requires a bit more to match.

[zero9178]

LGTM

mlir/lib/Target/LLVMIR/ModuleImport.cpp

LogicalResult ModuleImport::processFunction(llvm::Function *func) {
   // Process the debug intrinsics that require a delayed conversion after
   // everything else was converted.
   if (failed(processDebugIntrinsics()))
     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())
+    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 a non-empty debug expression.
   // TODO: Support debug intrinsics that evaluate a debug expression.
   if (dbgIntr->hasArgList() || dbgIntr->getExpression()->getNumElements() != 0)
     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();
   FailureOr<Value> argOperand = convertMetadataValue(dbgIntr->getArgOperand(0));
   if (failed(argOperand))
-    return failure();
+    return emitError(loc) << "failed to convert a debug intrinsic operand: "
+                          << diag(*dbgIntr);
 
   // 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>()) {

mlir/test/Target/LLVMIR/Import/import-failure.ll

 ; // -----
 
 declare void @llvm.dbg.value(metadata, metadata, metadata)
 
 ; CHECK:      import-failure.ll
 ; 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 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
+; CHECK:      import-failure.ll
+; CHECK-SAME: warning: dropped intrinsic: call void @llvm.dbg.value(metadata !6, metadata !3, metadata !DIExpression()), !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
+  call void @llvm.dbg.value(metadata !6, metadata !3, metadata !DIExpression()), !dbg !5
   ret void
 }
 
 !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: "import-failure.ll", directory: "/")
 !3 = !DILocalVariable(scope: !4, name: "arg1", file: !2, line: 1, arg: 1, align: 64);
 !4 = distinct !DISubprogram(name: "intrinsic", scope: !2, file: !2, spFlags: DISPFlagDefinition, unit: !1)
 !5 = !DILocation(line: 1, column: 2, scope: !4)
+!6 = !{}
 
 ; // -----
 
 ; global_dtors with non-null data fields cannot be represented in MLIR.
 ; CHECK:      <unknown>
 ; 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 }]