diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h index a4a7df985b681..dba950c0b48b6 100644 --- 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 *dbgRecord); /// Converts the LLVM values for an intrinsic to mixed MLIR values and /// attributes for LLVM_IntrOpBase. Attributes correspond to LLVM immargs. The @@ -343,14 +344,14 @@ class ModuleImport { /// Converts all debug intrinsics in `debugIntrinsics`. Assumes that the /// function containing the intrinsics has been fully converted to MLIR. LogicalResult processDebugIntrinsics(); - /// Converts all debug records in `debugRecords`. Assumes that the + /// Converts all debug records in `dbgRecords`. Assumes that the /// function containing the record has been fully converted to MLIR. LogicalResult processDebugRecords(); /// Converts a single debug intrinsic. LogicalResult processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr, DominanceInfo &domInfo); /// Converts a single debug record. - LogicalResult processDebugRecord(llvm::DbgRecord &debugRecord, + LogicalResult processDebugRecord(llvm::DbgVariableRecord &dbgRecord, DominanceInfo &domInfo); /// Process arguments for declare/value operation insertion. `localVarAttr` /// and `localExprAttr` are the attained attributes after importing the debug @@ -358,7 +359,7 @@ class ModuleImport { /// used by these operations. std::tuple processDebugOpArgumentsAndInsertionPt( - Location loc, bool hasArgList, bool isKillLocation, + Location loc, llvm::function_ref()> convertArgOperandToValue, llvm::Value *address, llvm::PointerUnion variable, @@ -508,7 +509,7 @@ class ModuleImport { SetVector debugIntrinsics; /// Function-local list of debug records that need to be imported after the /// function conversion has finished. - SetVector debugRecords; + SetVector dbgRecords; /// Mapping between LLVM alias scope and domain metadata nodes and /// attributes in the LLVM dialect corresponding to these nodes. DenseMap aliasScopeMapping; diff --git a/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp b/mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp index ba80f6294bd9b..5be33c4c78138 100644 --- 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 index 4d63e0534358f..18e132dfeabc2 100644 --- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp +++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp @@ -524,9 +524,9 @@ void ModuleImport::addDebugIntrinsic(llvm::CallInst *intrinsic) { debugIntrinsics.insert(intrinsic); } -void ModuleImport::addDebugRecord(llvm::DbgRecord *debugRecord) { - if (!debugRecords.contains(debugRecord)) - debugRecords.insert(debugRecord); +void ModuleImport::addDebugRecord(llvm::DbgVariableRecord *dbgRecord) { + if (!dbgRecords.contains(dbgRecord)) + dbgRecords.insert(dbgRecord); } static Attribute convertCGProfileModuleFlagValue(ModuleOp mlirModule, @@ -2557,10 +2557,40 @@ LogicalResult ModuleImport::processInstruction(llvm::Instruction *inst) { if (auto *intrinsic = dyn_cast(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 &dbgRecord : inst->DebugMarker->getDbgRecordRange()) { + // Store debug variable records for later processing. + if (auto *dbgVariableRecord = + dyn_cast(&dbgRecord)) { + addDebugRecord(dbgVariableRecord); + continue; + } + Location loc = translateLoc(dbgRecord.getDebugLoc()); + auto emitUnsupportedWarning = [&]() -> LogicalResult { + if (!emitExpensiveWarnings) + return success(); + std::string options; + llvm::raw_string_ostream optionsStream(options); + dbgRecord.print(optionsStream); + emitWarning(loc) << "unhandled debug record " << optionsStream.str(); + return success(); + }; + // Convert the debug label records in-place. + if (auto *dbgLabelRecord = dyn_cast(&dbgRecord)) { + DILabelAttr labelAttr = + debugImporter->translate(dbgLabelRecord->getLabel()); + if (!labelAttr) + return emitUnsupportedWarning(); + LLVM::DbgLabelOp::create(builder, loc, labelAttr); + continue; + } + // Warn if an unsupported debug record is encountered. + return emitUnsupportedWarning(); + } + } // Convert all remaining LLVM instructions to MLIR operations. return convertInstruction(inst); @@ -3047,10 +3077,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(value); if (!nodeAsVal) return false; @@ -3095,23 +3127,13 @@ static LogicalResult setDebugIntrinsicBuilderInsertionPoint( std::tuple ModuleImport::processDebugOpArgumentsAndInsertionPt( - Location loc, bool hasArgList, bool isKillLocation, + Location loc, llvm::function_ref()> convertArgOperandToValue, llvm::Value *address, llvm::PointerUnion 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 +3166,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 +3204,35 @@ ModuleImport::processDebugIntrinsic(llvm::DbgVariableIntrinsic *dbgIntr, return success(); } -LogicalResult ModuleImport::processDebugRecord(llvm::DbgRecord &debugRecord, - DominanceInfo &domInfo) { - Location loc = translateLoc(debugRecord.getDebugLoc()); - auto emitUnsupportedWarning = [&]() { +LogicalResult +ModuleImport::processDebugRecord(llvm::DbgVariableRecord &dbgRecord, + DominanceInfo &domInfo) { + OpBuilder::InsertionGuard guard(builder); + Location loc = translateLoc(dbgRecord.getDebugLoc()); + auto emitUnsupportedWarning = [&]() -> LogicalResult { if (!emitExpensiveWarnings) return success(); std::string options; llvm::raw_string_ostream optionsStream(options); - debugRecord.print(optionsStream); - emitWarning(loc) << "unhandled debug record " << optionsStream.str(); + dbgRecord.print(optionsStream); + emitWarning(loc) << "unhandled debug variable record " + << optionsStream.str(); return success(); }; - OpBuilder::InsertionGuard guard(builder); - auto *dbgVar = dyn_cast(&debugRecord); - if (!dbgVar) + // Drop debug records with an argument list. + // TODO: Support this case. + if (dbgRecord.hasArgList()) + return emitUnsupportedWarning(); + + // Drop all other debug records with a address operand that cannot be + // converted to an SSA value such as an empty metadata node. + // TODO: Support this case. + if (!dbgRecord.getAddress()) return emitUnsupportedWarning(); auto convertArgOperandToValue = [&]() -> FailureOr { - llvm::Value *value = dbgVar->getAddress(); + llvm::Value *value = dbgRecord.getAddress(); // Return the mapped value if it has been converted before. auto it = valueMapping.find(value); @@ -3205,9 +3247,8 @@ LogicalResult ModuleImport::processDebugRecord(llvm::DbgRecord &debugRecord, auto [localVariableAttr, locationExprAttr, locVal] = processDebugOpArgumentsAndInsertionPt( - loc, dbgVar->hasArgList(), dbgVar->isKillLocation(), - convertArgOperandToValue, dbgVar->getAddress(), dbgVar->getVariable(), - dbgVar->getExpression(), domInfo); + loc, convertArgOperandToValue, dbgRecord.getAddress(), + dbgRecord.getVariable(), dbgRecord.getExpression(), domInfo); if (!localVariableAttr) return emitUnsupportedWarning(); @@ -3215,10 +3256,10 @@ LogicalResult ModuleImport::processDebugRecord(llvm::DbgRecord &debugRecord, if (!locVal) // Expected if localVariableAttr is present. return failure(); - if (dbgVar->isDbgDeclare()) + if (dbgRecord.isDbgDeclare()) LLVM::DbgDeclareOp::create(builder, loc, locVal, localVariableAttr, locationExprAttr); - else if (dbgVar->isDbgValue()) + else if (dbgRecord.isDbgValue()) LLVM::DbgValueOp::create(builder, loc, locVal, localVariableAttr, locationExprAttr); else // isDbgAssign @@ -3239,11 +3280,10 @@ LogicalResult ModuleImport::processDebugIntrinsics() { LogicalResult ModuleImport::processDebugRecords() { DominanceInfo domInfo; - for (llvm::DbgRecord *debugRecord : debugRecords) { - if (failed(processDebugRecord(*debugRecord, domInfo))) + for (llvm::DbgVariableRecord *dbgRecord : dbgRecords) + if (failed(processDebugRecord(*dbgRecord, domInfo))) return failure(); - } - debugRecords.clear(); + dbgRecords.clear(); return success(); } diff --git a/mlir/test/Target/LLVMIR/Import/import-failure.ll b/mlir/test/Target/LLVMIR/Import/import-failure.ll index d48be66f2063e..32f730b545405 100644 --- 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 variable 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 variable 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 }