[Mlir-commits] [mlir] [MLIR][LLVM] Debug info: import debug records directly (PR #167812)

[Tobias Gysi]

================
@@ -3007,100 +3026,202 @@ LogicalResult ModuleImport::processFunction(llvm::Function *func) {
   if (failed(processDebugIntrinsics()))
     return failure();
 
+  // Process the debug r that require a delayed conversion after
+  // everything else was converted.
+  if (failed(processDebugRecords()))
+    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())
+/// Checks if a kill location holds metadata instead of an SSA value.
+static bool isMetadataKillLocation(bool isKillLocation, llvm::Value *value) {
+  if (!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 arg lists.
-  // TODO: Support debug intrinsics that have arg lists.
-  if (dbgIntr->hasArgList())
-    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();
-  // Drop debug intrinsics if the associated variable information cannot be
-  // translated due to cyclic debug metadata.
-  // TODO: Support cyclic debug metadata.
-  DILocalVariableAttr localVariableAttr =
-      matchLocalVariableAttr(dbgIntr->getArgOperand(1));
-  if (!localVariableAttr)
-    return emitUnsupportedWarning();
-  FailureOr<Value> argOperand = convertMetadataValue(dbgIntr->getArgOperand(0));
-  if (failed(argOperand))
-    return emitError(loc) << "failed to convert a debug intrinsic operand: "
-                          << diag(*dbgIntr);
-
-  // Ensure that the debug intrinsic 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();
+/// Ensure that the debug intrinsic 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.
+static LogicalResult setDebugIntrinsicBuilderInsertionPoint(
+    mlir::OpBuilder &builder, DominanceInfo &domInfo, Value argOperand) {
+  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();
+      return failure();
     // Set insertion point before the terminator, to avoid inserting something
     // before landingpads.
     Block *dominatedBlock = (*dominatedBlocks.begin())->getBlock();
     builder.setInsertionPoint(dominatedBlock->getTerminator());
   } else {
-    Value insertPt = *argOperand;
-    if (auto blockArg = dyn_cast<BlockArgument>(*argOperand)) {
+    Value insertPt = argOperand;
+    if (auto blockArg = dyn_cast<BlockArgument>(argOperand)) {
       // The value might be coming from a phi node and is now a block argument,
       // which means the insertion point is set to the start of the block. If
       // this block is a target destination of an invoke, the insertion point
       // must happen after the landing pad operation.
-      Block *insertionBlock = argOperand->getParentBlock();
+      Block *insertionBlock = argOperand.getParentBlock();
       if (!insertionBlock->empty() &&
           isa<LandingpadOp>(insertionBlock->front()))
         insertPt = cast<LandingpadOp>(insertionBlock->front()).getRes();
     }
 
     builder.setInsertionPointAfterValue(insertPt);
   }
-  auto locationExprAttr =
-      debugImporter->translateExpression(dbgIntr->getExpression());
+  return success();
+}
+
+void ModuleImport::processDebugOpArgumentsAndInsertionPt(
+    Location loc, DILocalVariableAttr &localVarAttr,
+    DIExpressionAttr &localExprAttr, Value &locVal, bool hasArgList,
+    bool isKillLocation,
+    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.
+  localVarAttr = matchLocalVariableAttr(variable);
+  if (!localVarAttr)
+    return;
+  FailureOr<Value> argOperand = convertArgOperandToValue();
+  if (failed(argOperand)) {
+    emitError(loc) << "failed to convert a debug operand: " << diag(*address);
+    return;
+  }
+
+  if (setDebugIntrinsicBuilderInsertionPoint(builder, domInfo, *argOperand)
+          .failed())
+    return;
+
+  localExprAttr = debugImporter->translateExpression(expression);
+  locVal = *argOperand;
+}
+
+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();
+  };
+
+  DILocalVariableAttr localVariableAttr;
+  DIExpressionAttr locationExprAttr;
+  Value locVal;
+  OpBuilder::InsertionGuard guard(builder);
+  auto convertArgOperandToValue = [&]() {
+    return convertMetadataValue(dbgIntr->getArgOperand(0));
+  };
+
+  processDebugOpArgumentsAndInsertionPt(
+      loc, localVariableAttr, locationExprAttr, locVal, dbgIntr->hasArgList(),
+      dbgIntr->isKillLocation(), convertArgOperandToValue,
+      dbgIntr->getArgOperand(0), dbgIntr->getArgOperand(1),
+      dbgIntr->getExpression(), domInfo);
+
+  if (!localVariableAttr)
+    return emitUnsupportedWarning();
+
+  if (!locVal) // Expected if localVariableAttr is present.
+    return failure();
+
   Operation *op =
       llvm::TypeSwitch<llvm::DbgVariableIntrinsic *, Operation *>(dbgIntr)
           .Case([&](llvm::DbgDeclareInst *) {
             return LLVM::DbgDeclareOp::create(
-                builder, loc, *argOperand, localVariableAttr, locationExprAttr);
+                builder, loc, locVal, localVariableAttr, locationExprAttr);
           })
           .Case([&](llvm::DbgValueInst *) {
             return LLVM::DbgValueOp::create(
-                builder, loc, *argOperand, localVariableAttr, locationExprAttr);
+                builder, loc, locVal, localVariableAttr, locationExprAttr);
           });
   mapNoResultOp(dbgIntr, op);
   setNonDebugMetadataAttrs(dbgIntr, op);
   return success();
 }
 
+LogicalResult ModuleImport::processDebugRecord(llvm::DbgRecord &dr,
----------------
gysit wrote:

```suggestion
LogicalResult ModuleImport::processDebugRecord(llvm::DbgRecord &debugRecord,
```
nit:

https://github.com/llvm/llvm-project/pull/167812