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

gysit · gysit · commit 8fda2cf39003 · 2025-11-20T09:20:09.000+01:00
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. - Add support for debug label records. This is a follow-up to: #167812
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -282,8 +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 *debugRecord);
+  /// Adds a debug record to the list of debug records that need to be imported
+  /// after the function conversion has finished.
+  void addDebugRecord(llvm::DbgVariableRecord *debugRecord);
 
   /// Converts the LLVM values for an intrinsic to mixed MLIR values and
   /// attributes for LLVM_IntrOpBase. Attributes correspond to LLVM immargs. The
@@ -349,16 +350,19 @@ class ModuleImport {
   /// Converts a single debug intrinsic.
   LogicalResult processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr,
                                       DominanceInfo &domInfo);
+  /// Emits a warning for an unsupported debug records.
+  LogicalResult emitUnsupportedDebugRecordWarning(llvm::DbgRecord &debugRecord,
+                                                  Location loc);
   /// Converts a single debug record.
-  LogicalResult processDebugRecord(llvm::DbgRecord &debugRecord,
+  LogicalResult processDebugRecord(llvm::DbgVariableRecord &debugRecord,
                                    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.
   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,
@@ -508,7 +512,7 @@ class ModuleImport {
   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;
+  SetVector<llvm::DbgVariableRecord *> 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
@@ -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(
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -524,7 +524,7 @@ void ModuleImport::addDebugIntrinsic(llvm::CallInst *intrinsic) {
   debugIntrinsics.insert(intrinsic);
 }
 
-void ModuleImport::addDebugRecord(llvm::DbgRecord *debugRecord) {
+void ModuleImport::addDebugRecord(llvm::DbgVariableRecord *debugRecord) {
   if (!debugRecords.contains(debugRecord))
     debugRecords.insert(debugRecord);
 }
@@ -2557,10 +2557,32 @@ 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 &debugRecord : inst->DebugMarker->getDbgRecordRange())
-      addDebugRecord(&debugRecord);
+  // Process debug records attached to this instruction. Debug variable records
+  // are stored for later processing after all SSA values are converted, while
+  // debug label records can be converted immediately.
+  if (inst->DebugMarker) {
+    for (llvm::DbgRecord &debugRecord :
+         inst->DebugMarker->getDbgRecordRange()) {
+      // Store debug variable records for later processing.
+      if (auto *dbgVariableRecord =
+              dyn_cast<llvm::DbgVariableRecord>(&debugRecord)) {
+        addDebugRecord(dbgVariableRecord);
+        continue;
+      }
+      Location loc = translateLoc(debugRecord.getDebugLoc());
+      // Convert debug labels in-place.
+      if (auto *dbgLabelRecord = dyn_cast<llvm::DbgLabelRecord>(&debugRecord)) {
+        DILabelAttr labelAttr =
+            debugImporter->translate(dbgLabelRecord->getLabel());
+        if (!labelAttr)
+          return emitUnsupportedDebugRecordWarning(debugRecord, loc);
+        LLVM::DbgLabelOp::create(builder, loc, labelAttr);
+        continue;
+      }
+      // Warn if an unsupported debug record is encountered.
+      return emitUnsupportedDebugRecordWarning(debugRecord, loc);
+    }
+  }
 
   // Convert all remaining LLVM instructions to MLIR operations.
   return convertInstruction(inst);
@@ -3047,10 +3069,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 +3119,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 +3158,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)
@@ -3171,26 +3196,26 @@ ModuleImport::processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr,
   return success();
 }
 
-LogicalResult ModuleImport::processDebugRecord(llvm::DbgRecord &debugRecord,
-                                               DominanceInfo &domInfo) {
-  Location loc = translateLoc(debugRecord.getDebugLoc());
-  auto emitUnsupportedWarning = [&]() {
-    if (!emitExpensiveWarnings)
-      return success();
-    std::string options;
-    llvm::raw_string_ostream optionsStream(options);
-    debugRecord.print(optionsStream);
-    emitWarning(loc) << "unhandled debug record " << optionsStream.str();
+LogicalResult
+ModuleImport::emitUnsupportedDebugRecordWarning(llvm::DbgRecord &debugRecord,
+                                                Location loc) {
+  if (!emitExpensiveWarnings)
     return success();
-  };
+  std::string options;
+  llvm::raw_string_ostream optionsStream(options);
+  debugRecord.print(optionsStream);
+  emitWarning(loc) << "unhandled debug record " << optionsStream.str();
+  return success();
+}
 
+LogicalResult
+ModuleImport::processDebugRecord(llvm::DbgVariableRecord &debugRecord,
+                                 DominanceInfo &domInfo) {
   OpBuilder::InsertionGuard guard(builder);
-  auto *dbgVar = dyn_cast<llvm::DbgVariableRecord>(&debugRecord);
-  if (!dbgVar)
-    return emitUnsupportedWarning();
+  Location loc = translateLoc(debugRecord.getDebugLoc());
 
   auto convertArgOperandToValue = [&]() -> FailureOr<Value> {
-    llvm::Value *value = dbgVar->getAddress();
+    llvm::Value *value = debugRecord.getAddress();
 
     // Return the mapped value if it has been converted before.
     auto it = valueMapping.find(value);
@@ -3203,26 +3228,36 @@ LogicalResult ModuleImport::processDebugRecord(llvm::DbgRecord &debugRecord,
     return failure();
   };
 
+  // Drop debug records with an argument list.
+  // TODO: Support this case.
+  if (debugRecord.hasArgList())
+    return emitUnsupportedDebugRecordWarning(debugRecord, loc);
+
+  // 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 (!debugRecord.getAddress())
+    return emitUnsupportedDebugRecordWarning(debugRecord, loc);
+
   auto [localVariableAttr, locationExprAttr, locVal] =
       processDebugOpArgumentsAndInsertionPt(
-          loc, dbgVar->hasArgList(), dbgVar->isKillLocation(),
-          convertArgOperandToValue, dbgVar->getAddress(), dbgVar->getVariable(),
-          dbgVar->getExpression(), domInfo);
+          loc, convertArgOperandToValue, debugRecord.getAddress(),
+          debugRecord.getVariable(), debugRecord.getExpression(), domInfo);
 
   if (!localVariableAttr)
-    return emitUnsupportedWarning();
+    return emitUnsupportedDebugRecordWarning(debugRecord, loc);
 
   if (!locVal) // Expected if localVariableAttr is present.
     return failure();
 
-  if (dbgVar->isDbgDeclare())
+  if (debugRecord.isDbgDeclare())
     LLVM::DbgDeclareOp::create(builder, loc, locVal, localVariableAttr,
                                locationExprAttr);
-  else if (dbgVar->isDbgValue())
+  else if (debugRecord.isDbgValue())
     LLVM::DbgValueOp::create(builder, loc, locVal, localVariableAttr,
                              locationExprAttr);
   else // isDbgAssign
-    return emitUnsupportedWarning();
+    return emitUnsupportedDebugRecordWarning(debugRecord, loc);
 
   return success();
 }
@@ -3239,10 +3274,9 @@ LogicalResult ModuleImport::processDebugIntrinsics() {
 
 LogicalResult ModuleImport::processDebugRecords() {
   DominanceInfo domInfo;
-  for (llvm::DbgRecord *debugRecord : debugRecords) {
+  for (llvm::DbgVariableRecord *debugRecord : debugRecords)
     if (failed(processDebugRecord(*debugRecord, domInfo)))
       return failure();
-  }
   debugRecords.clear();
   return success();
 }
diff --git a/mlir/test/Target/LLVMIR/Import/import-failure.ll b/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
 }
 
