[mlir][llvm] Handle debug record import edge cases

This commit enables the direct import of debug records by default and
fixes issues with two edge cases:
- Detect early on if the address operand is an argument list
  (calling getAddress() for argument lists asserts)
- Use getAddress() to check if the address operand is null, which
  means the address operand is an empty metadata node, which currently
  is not supported.

This is a follow-up to:
#167812

Author: gysit

mlir/include/mlir/Target/LLVMIR/ModuleImport.h

@@ -358,7 +358,7 @@ class ModuleImport {
   /// used by these operations.
   std::tuple<DILocalVariableAttr, DIExpressionAttr, Value>
   processDebugOpArgumentsAndInsertionPt(
-      Location loc, bool hasArgList, bool isKillLocation,
+      Location loc,
       llvm::function_ref<FailureOr<Value>()> convertArgOperandToValue,
       llvm::Value *address,
       llvm::PointerUnion<llvm::Value *, llvm::DILocalVariable *> variable,

mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

@@ -79,7 +79,7 @@ void registerFromLLVMIRTranslation() {
 
         // Now that the translation supports importing debug records directly,
         // make it the default, but allow the user to override to old behavior.
-        if (!convertDebugRecToIntrinsics)
+        if (convertDebugRecToIntrinsics)
           llvmModule->convertFromNewDbgValues();
 
         return translateLLVMIRToModule(

mlir/lib/Target/LLVMIR/ModuleImport.cpp

@@ -3047,10 +3047,12 @@ LogicalResult ModuleImport::processFunction(llvm::Function *func) {
   return success();
 }
 
-/// Checks if a kill location holds metadata instead of an SSA value.
-static bool isMetadataKillLocation(bool isKillLocation, llvm::Value *value) {
-  if (!isKillLocation)
+/// 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;
@@ -3095,23 +3097,13 @@ static LogicalResult setDebugIntrinsicBuilderInsertionPoint(
 
 std::tuple<DILocalVariableAttr, DIExpressionAttr, Value>
 ModuleImport::processDebugOpArgumentsAndInsertionPt(
-    Location loc, bool hasArgList, bool isKillLocation,
+    Location loc,
     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.
+  // Drop debug intrinsics if the associated debug information cannot be
+  // translated due to an unsupported construct.
   DILocalVariableAttr localVarAttr = matchLocalVariableAttr(variable);
   if (!localVarAttr)
     return {};
@@ -3144,10 +3136,21 @@ ModuleImport::processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr,
     return convertMetadataValue(dbgIntr->getArgOperand(0));
   };
 
+  // Drop debug intrinsics with an argument list.
+  // TODO: Support this case.
+  if (dbgIntr->hasArgList())
+    return emitUnsupportedWarning();
+
+  // Drop debug intrinsics with kill locations that have metadata nodes as
+  // location operand, which cannot be converted to poison as the type cannot be
+  // reconstructed.
+  // TODO: Support this case.
+  if (isMetadataKillLocation(dbgIntr))
+    return emitUnsupportedWarning();
+
   auto [localVariableAttr, locationExprAttr, locVal] =
       processDebugOpArgumentsAndInsertionPt(
-          loc, dbgIntr->hasArgList(), dbgIntr->isKillLocation(),
-          convertArgOperandToValue, dbgIntr->getArgOperand(0),
+          loc, convertArgOperandToValue, dbgIntr->getArgOperand(0),
           dbgIntr->getArgOperand(1), dbgIntr->getExpression(), domInfo);
 
   if (!localVariableAttr)
@@ -3203,11 +3206,21 @@ LogicalResult ModuleImport::processDebugRecord(llvm::DbgRecord &debugRecord,
     return failure();
   };
 
+  // Drop debug records with an argument list.
+  // TODO: Support this case.
+  if (dbgVar->hasArgList())
+    return emitUnsupportedWarning();
+
+  // Drop kill location debug records with a null address operand, which cannot
+  // be converted to poison as the type cannot be reconstructed.
+  // TODO: Support this case.
+  if (!dbgVar->getAddress())
+    return emitUnsupportedWarning();
+
   auto [localVariableAttr, locationExprAttr, locVal] =
       processDebugOpArgumentsAndInsertionPt(
-          loc, dbgVar->hasArgList(), dbgVar->isKillLocation(),
-          convertArgOperandToValue, dbgVar->getAddress(), dbgVar->getVariable(),
-          dbgVar->getExpression(), domInfo);
+          loc, convertArgOperandToValue, dbgVar->getAddress(),
+          dbgVar->getVariable(), dbgVar->getExpression(), domInfo);
 
   if (!localVariableAttr)
     return emitUnsupportedWarning();

mlir/test/Target/LLVMIR/Import/debug-info.ll

@@ -1,4 +1,4 @@
-; RUN: mlir-translate -import-llvm -mlir-print-debuginfo -split-input-file %s | FileCheck %s
+; RUN: mlir-translate -import-llvm -convert-debug-rec-to-intrinsics -mlir-print-debuginfo -split-input-file %s | FileCheck %s
 
 ; CHECK: #[[$UNKNOWN_LOC:.+]] = loc(unknown)
 

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

@@ -1,16 +1,14 @@
 ; RUN: not mlir-translate -import-llvm -emit-expensive-warnings -split-input-file %s 2>&1 -o /dev/null | FileCheck %s
 
-; Check that debug intrinsics with an unsupported argument are dropped.
-
-declare void @llvm.dbg.value(metadata, metadata, metadata)
+; Check that debug records with an unsupported argument are dropped.
 
 ; CHECK:      import-failure.ll
-; CHECK-SAME: warning: dropped intrinsic: tail call void @llvm.dbg.value(metadata !DIArgList(i64 %{{.*}}, 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))
+; CHECK-SAME: warning: unhandled debug record #dbg_value(!DIArgList(i64 %{{.*}}, i64 undef), !{{.*}}, !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), !{{.*}})
 ; CHECK:      import-failure.ll
-; CHECK-SAME: warning: dropped intrinsic: tail call void @llvm.dbg.value(metadata !6, metadata !3, metadata !DIExpression())
+; CHECK-SAME: warning: unhandled debug record #dbg_value(!{{.*}}, !{{.*}}, !DIExpression(), !{{.*}})
 define void @unsupported_argument(i64 %arg1) {
-  tail 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
-  tail call void @llvm.dbg.value(metadata !6, metadata !3, metadata !DIExpression()), !dbg !5
+  #dbg_value(!DIArgList(i64 %arg1, i64 undef), !3, !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), !5)
+  #dbg_value(!6, !3, !DIExpression(), !5)
   ret void
 }