[mlir][debug] Allow multiple DIGlobalVariableExpression on globals. #111981
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, we allow only one DIGlobalVariableExpressionAttr per global. Also when we are importing, we just pick the first one. In contrast, LLVM allows multiple DIGlobalVariableExpression to be attached to the global. They are needed for correct working of things like DICommonBlock. This PR removes this restriction in mlir. The changes are mostly mechanical. The testcases have been adjusted accordingly.
llvmbot
added
mlir:llvm
mlir
flang
|
@llvm/pr-subscribers-flang-codegen @llvm/pr-subscribers-flang-fir-hlfir Author: Abid Qadeer (abidh) ChangesCurrently, we allow only one DIGlobalVariableExpressionAttr per global. It is especially evident in import where we pick the first from the list and ignore the rest. In contrast, LLVM allows multiple DIGlobalVariableExpression to be attached to the global. They are needed for correct working of things like DICommonBlock. This PR removes this restriction in mlir. Changes are mostly mechanical. One thing on which I went a bit back and forth was the representation inside GlobalOp. I would be happy to change if there are better ways to do this. Patch is 29.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/111981.diff 11 Files Affected:
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 15fcc09c6219a2..9b624efa053813 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -2808,14 +2808,14 @@ struct GlobalOpConversion : public fir::FIROpConversion<fir::GlobalOp> {
matchAndRewrite(fir::GlobalOp global, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const override {
- mlir::LLVM::DIGlobalVariableExpressionAttr dbgExpr;
+ llvm::SmallVector<mlir::Attribute> dbgExprs;
if (auto fusedLoc = mlir::dyn_cast<mlir::FusedLoc>(global.getLoc())) {
if (auto gvAttr =
mlir::dyn_cast_or_null<mlir::LLVM::DIGlobalVariableAttr>(
fusedLoc.getMetadata())) {
- dbgExpr = mlir::LLVM::DIGlobalVariableExpressionAttr::get(
- global.getContext(), gvAttr, mlir::LLVM::DIExpressionAttr());
+ dbgExprs.push_back(mlir::LLVM::DIGlobalVariableExpressionAttr::get(
+ global.getContext(), gvAttr, mlir::LLVM::DIExpressionAttr()));
}
}
@@ -2831,7 +2831,7 @@ struct GlobalOpConversion : public fir::FIROpConversion<fir::GlobalOp> {
llvm::ArrayRef<mlir::NamedAttribute> attrs;
auto g = rewriter.create<mlir::LLVM::GlobalOp>(
loc, tyAttr, isConst, linkage, global.getSymName(), initAttr, 0, 0,
- false, false, comdat, attrs, dbgExpr);
+ false, false, comdat, attrs, dbgExprs);
if (global.getAlignment() && *global.getAlignment() > 0)
g.setAlignment(*global.getAlignment());
diff --git a/flang/test/Transforms/debug-module-2.fir b/flang/test/Transforms/debug-module-2.fir
index 6acdc1df23d27c..c8d618ce34b266 100644
--- a/flang/test/Transforms/debug-module-2.fir
+++ b/flang/test/Transforms/debug-module-2.fir
@@ -31,5 +31,5 @@ module {
// CHECK-DAG: #[[GLR:.*]] = #llvm.di_global_variable<{{.*}}name = "glr", linkageName = "_QMhelperEglr"{{.*}}>
// CHECK-DAG: #[[GLIE:.*]] = #llvm.di_global_variable_expression<var = #[[GLI]]>
// CHECK-DAG: #[[GLRE:.*]] = #llvm.di_global_variable_expression<var = #[[GLR]]>
-// CHECK-DAG: llvm.mlir.global{{.*}}@_QMhelperEgli() {{{.*}}dbg_expr = #[[GLIE]]}
-// CHECK-DAG: llvm.mlir.global{{.*}}@_QMhelperEglr() {{{.*}}dbg_expr = #[[GLRE]]}
+// CHECK-DAG: llvm.mlir.global{{.*}}@_QMhelperEgli() {{{.*}}dbg_exprs = [#[[GLIE]]]}
+// CHECK-DAG: llvm.mlir.global{{.*}}@_QMhelperEglr() {{{.*}}dbg_exprs = [#[[GLRE]]]}
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 000d92f9ea3bcb..3cddcb3e23784a 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -16,6 +16,7 @@
include "mlir/Dialect/LLVMIR/LLVMAttrDefs.td"
include "mlir/Dialect/LLVMIR/LLVMEnums.td"
include "mlir/Dialect/LLVMIR/LLVMOpBase.td"
+include "mlir/IR/AttrTypeBase.td"
include "mlir/IR/EnumAttr.td"
include "mlir/Interfaces/FunctionInterfaces.td"
include "mlir/IR/SymbolInterfaces.td"
@@ -1152,6 +1153,9 @@ def LLVM_AddressOfOp : LLVM_Op<"mlir.addressof",
let hasFolder = 1;
}
+def DIGlobalVariableExpressionArrayAttr :
+ TypedArrayAttrBase<LLVM_DIGlobalVariableExpressionAttr, "an array of variable expressions">;
+
def LLVM_GlobalOp : LLVM_Op<"mlir.global",
[IsolatedFromAbove, SingleBlockImplicitTerminator<"ReturnOp">, Symbol]> {
let arguments = (ins
@@ -1168,7 +1172,7 @@ def LLVM_GlobalOp : LLVM_Op<"mlir.global",
OptionalAttr<UnnamedAddr>:$unnamed_addr,
OptionalAttr<StrAttr>:$section,
OptionalAttr<SymbolRefAttr>:$comdat,
- DefaultValuedAttr<LLVM_DIGlobalVariableExpressionAttr, "{}">:$dbg_expr,
+ OptionalAttr<DIGlobalVariableExpressionArrayAttr>:$dbg_exprs,
DefaultValuedAttr<Visibility, "mlir::LLVM::Visibility::Default">:$visibility_
);
let summary = "LLVM dialect global.";
@@ -1279,7 +1283,7 @@ def LLVM_GlobalOp : LLVM_Op<"mlir.global",
CArg<"bool", "false">:$thread_local_,
CArg<"SymbolRefAttr", "{}">:$comdat,
CArg<"ArrayRef<NamedAttribute>", "{}">:$attrs,
- CArg<"DIGlobalVariableExpressionAttr", "{}">:$dbgExpr)>
+ CArg<"ArrayRef<Attribute>", "{}">:$dbgExprs)>
];
let extraClassDeclaration = [{
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 006d412936a337..55c4e03aaefe7b 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2089,7 +2089,7 @@ void GlobalOp::build(OpBuilder &builder, OperationState &result, Type type,
Attribute value, uint64_t alignment, unsigned addrSpace,
bool dsoLocal, bool threadLocal, SymbolRefAttr comdat,
ArrayRef<NamedAttribute> attrs,
- DIGlobalVariableExpressionAttr dbgExpr) {
+ ArrayRef<Attribute> dbgExprs) {
result.addAttribute(getSymNameAttrName(result.name),
builder.getStringAttr(name));
result.addAttribute(getGlobalTypeAttrName(result.name), TypeAttr::get(type));
@@ -2121,8 +2121,9 @@ void GlobalOp::build(OpBuilder &builder, OperationState &result, Type type,
builder.getI32IntegerAttr(addrSpace));
result.attributes.append(attrs.begin(), attrs.end());
- if (dbgExpr)
- result.addAttribute(getDbgExprAttrName(result.name), dbgExpr);
+ if (!dbgExprs.empty())
+ result.addAttribute(getDbgExprsAttrName(result.name),
+ ArrayAttr::get(builder.getContext(), dbgExprs));
result.addRegion();
}
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 4ff1f1135b0a88..7a164e95c840f5 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -914,14 +914,16 @@ LogicalResult ModuleImport::convertGlobal(llvm::GlobalVariable *globalVar) {
// Get the global expression associated with this global variable and convert
// it.
- DIGlobalVariableExpressionAttr globalExpressionAttr;
+ SmallVector<Attribute> globalExpressionAttrs;
SmallVector<llvm::DIGlobalVariableExpression *> globalExpressions;
globalVar->getDebugInfo(globalExpressions);
// There should only be a single global expression.
- if (!globalExpressions.empty())
- globalExpressionAttr =
- debugImporter->translateGlobalVariableExpression(globalExpressions[0]);
+ for (auto expr : globalExpressions) {
+ DIGlobalVariableExpressionAttr globalExpressionAttr =
+ debugImporter->translateGlobalVariableExpression(expr);
+ globalExpressionAttrs.push_back(globalExpressionAttr);
+ }
// Workaround to support LLVM's nameless globals. MLIR, in contrast to LLVM,
// always requires a symbol name.
@@ -935,7 +937,7 @@ LogicalResult ModuleImport::convertGlobal(llvm::GlobalVariable *globalVar) {
valueAttr, alignment, /*addr_space=*/globalVar->getAddressSpace(),
/*dso_local=*/globalVar->isDSOLocal(),
/*thread_local=*/globalVar->isThreadLocal(), /*comdat=*/SymbolRefAttr(),
- /*attrs=*/ArrayRef<NamedAttribute>(), /*dbgExpr=*/globalExpressionAttr);
+ /*attrs=*/ArrayRef<NamedAttribute>(), /*dbgExprs=*/globalExpressionAttrs);
globalInsertionOp = globalOp;
if (globalVar->hasInitializer() && !valueAttr) {
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index a5de90160c4145..6430dd5bca3ee2 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -1055,44 +1055,50 @@ LogicalResult ModuleTranslation::convertGlobals() {
globalsMapping.try_emplace(op, var);
// Add debug information if present.
- if (op.getDbgExpr()) {
- llvm::DIGlobalVariableExpression *diGlobalExpr =
- debugTranslation->translateGlobalVariableExpression(op.getDbgExpr());
- llvm::DIGlobalVariable *diGlobalVar = diGlobalExpr->getVariable();
- var->addDebugInfo(diGlobalExpr);
-
- // There is no `globals` field in DICompileUnitAttr which can be directly
- // assigned to DICompileUnit. We have to build the list by looking at the
- // dbgExpr of all the GlobalOps. The scope of the variable is used to get
- // the DICompileUnit in which to add it.
- // But there are cases where the scope of a global does not
- // directly point to the DICompileUnit and we have to do a bit more work
- // to get to it. Some of those cases are:
- //
- // 1. For the languages that support modules, the scope hierarchy can be
- // variable -> DIModule -> DICompileUnit
- //
- // 2. For the Fortran common block variable, the scope hierarchy can be
- // variable -> DICommonBlock -> DISubprogram -> DICompileUnit
- //
- // 3. For entities like static local variables in C or variable with
- // SAVE attribute in Fortran, the scope hierarchy can be
- // variable -> DISubprogram -> DICompileUnit
- llvm::DIScope *scope = diGlobalVar->getScope();
- if (auto *mod = dyn_cast_if_present<llvm::DIModule>(scope))
- scope = mod->getScope();
- else if (auto *cb = dyn_cast_if_present<llvm::DICommonBlock>(scope)) {
- if (auto *sp = dyn_cast_if_present<llvm::DISubprogram>(cb->getScope()))
- scope = sp->getUnit();
- } else if (auto *sp = dyn_cast_if_present<llvm::DISubprogram>(scope))
- scope = sp->getUnit();
-
- // Get the compile unit (scope) of the the global variable.
- if (llvm::DICompileUnit *compileUnit =
- dyn_cast_if_present<llvm::DICompileUnit>(scope)) {
- // Update the compile unit with this incoming global variable expression
- // during the finalizing step later.
- allGVars[compileUnit].push_back(diGlobalExpr);
+ if (op.getDbgExprs()) {
+ for (auto attr : *op.getDbgExprs()) {
+ if (auto exprAttr =
+ dyn_cast_if_present<DIGlobalVariableExpressionAttr>(attr)) {
+ llvm::DIGlobalVariableExpression *diGlobalExpr =
+ debugTranslation->translateGlobalVariableExpression(exprAttr);
+ llvm::DIGlobalVariable *diGlobalVar = diGlobalExpr->getVariable();
+ var->addDebugInfo(diGlobalExpr);
+
+ // There is no `globals` field in DICompileUnitAttr which can be
+ // directly assigned to DICompileUnit. We have to build the list by
+ // looking at the dbgExpr of all the GlobalOps. The scope of the
+ // variable is used to get the DICompileUnit in which to add it. But
+ // there are cases where the scope of a global does not directly point
+ // to the DICompileUnit and we have to do a bit more work to get to
+ // it. Some of those cases are:
+ //
+ // 1. For the languages that support modules, the scope hierarchy can
+ // be variable -> DIModule -> DICompileUnit
+ //
+ // 2. For the Fortran common block variable, the scope hierarchy can
+ // be variable -> DICommonBlock -> DISubprogram -> DICompileUnit
+ //
+ // 3. For entities like static local variables in C or variable with
+ // SAVE attribute in Fortran, the scope hierarchy can be
+ // variable -> DISubprogram -> DICompileUnit
+ llvm::DIScope *scope = diGlobalVar->getScope();
+ if (auto *mod = dyn_cast_if_present<llvm::DIModule>(scope))
+ scope = mod->getScope();
+ else if (auto *cb = dyn_cast_if_present<llvm::DICommonBlock>(scope)) {
+ if (auto *sp =
+ dyn_cast_if_present<llvm::DISubprogram>(cb->getScope()))
+ scope = sp->getUnit();
+ } else if (auto *sp = dyn_cast_if_present<llvm::DISubprogram>(scope))
+ scope = sp->getUnit();
+
+ // Get the compile unit (scope) of the the global variable.
+ if (llvm::DICompileUnit *compileUnit =
+ dyn_cast_if_present<llvm::DICompileUnit>(scope)) {
+ // Update the compile unit with this incoming global variable
+ // expression during the finalizing step later.
+ allGVars[compileUnit].push_back(diGlobalExpr);
+ }
+ }
}
}
}
diff --git a/mlir/test/Dialect/LLVMIR/debuginfo.mlir b/mlir/test/Dialect/LLVMIR/debuginfo.mlir
index 8475ec6c3510db..dafb3bcef740f0 100644
--- a/mlir/test/Dialect/LLVMIR/debuginfo.mlir
+++ b/mlir/test/Dialect/LLVMIR/debuginfo.mlir
@@ -162,7 +162,18 @@
file = #file, line = 2, type = #int0>
#var_expression = #llvm.di_global_variable_expression<var = #global_var,
expr = <>>
-llvm.mlir.global common @block_() {dbg_expr = #var_expression} : i64
+#global_var1 = #llvm.di_global_variable<scope = #di_common_block, name = "b",
+ file = #file, line = 3, type = #int0>
+#var_expression1 = #llvm.di_global_variable_expression<var = #global_var1,
+ expr = <>>
+llvm.mlir.global @data() {dbg_exprs = [#var_expression, #var_expression1]} : i64
+
+// CHECK-DAG: llvm.mlir.global external @data() {{{.*}}dbg_exprs = [#[[EXP1:.*]], #[[EXP2:.*]]]} : i64
+// CHECK-DAG: #[[EXP1]] = #llvm.di_global_variable_expression<var = #[[GV1:.*]], expr = <>>
+// CHECK-DAG: #[[EXP2]] = #llvm.di_global_variable_expression<var = #[[GV2:.*]], expr = <>>
+// CHECK-DAG: #[[GV1]] = #llvm.di_global_variable<{{.*}}name = "a"{{.*}}>
+// CHECK-DAG: #[[GV2]] = #llvm.di_global_variable<{{.*}}name = "b"{{.*}}>
+
// CHECK: llvm.func @addr(%[[ARG:.*]]: i64)
llvm.func @addr(%arg: i64) {
diff --git a/mlir/test/Dialect/LLVMIR/global.mlir b/mlir/test/Dialect/LLVMIR/global.mlir
index 3fa7636d4dd686..79d1cafabfbed3 100644
--- a/mlir/test/Dialect/LLVMIR/global.mlir
+++ b/mlir/test/Dialect/LLVMIR/global.mlir
@@ -272,15 +272,15 @@ llvm.mlir.global @target_fail(0 : i64) : !llvm.target<"spirv.Image", i32, 0>
// CHECK-DAG: #[[EXPR1:.*]] = #llvm.di_global_variable_expression<var = #[[GVAR1]], expr = <[DW_OP_push_object_address, DW_OP_deref]>>
// CHECK-DAG: #[[EXPR2:.*]] = #llvm.di_global_variable_expression<var = #[[GVAR2]], expr = <[DW_OP_LLVM_arg(0), DW_OP_LLVM_arg(1), DW_OP_plus]>>
// CHECK-DAG: #[[EXPR3:.*]] = #llvm.di_global_variable_expression<var = #[[GVAR3]], expr = <[DW_OP_LLVM_convert(16, DW_ATE_signed)]>>
-// CHECK-DAG: llvm.mlir.global external @global_with_expr1() {addr_space = 0 : i32, dbg_expr = #[[EXPR0]]} : i64
-// CHECK-DAG: llvm.mlir.global external @global_with_expr2() {addr_space = 0 : i32, dbg_expr = #[[EXPR1]]} : i64
-// CHECK-DAG: llvm.mlir.global external @global_with_expr3() {addr_space = 0 : i32, dbg_expr = #[[EXPR2]]} : i64
-// CHECK-DAG: llvm.mlir.global external @global_with_expr4() {addr_space = 0 : i32, dbg_expr = #[[EXPR3]]} : i64
+// CHECK-DAG: llvm.mlir.global external @global_with_expr1() {addr_space = 0 : i32, dbg_expr = [#[[EXPR0]]]} : i64
+// CHECK-DAG: llvm.mlir.global external @global_with_expr2() {addr_space = 0 : i32, dbg_expr = [#[[EXPR1]]]} : i64
+// CHECK-DAG: llvm.mlir.global external @global_with_expr3() {addr_space = 0 : i32, dbg_expr = [#[[EXPR2]]]} : i64
+// CHECK-DAG: llvm.mlir.global external @global_with_expr4() {addr_space = 0 : i32, dbg_expr = [#[[EXPR3]]]} : i64
#di_file = #llvm.di_file<"not" in "existence">
#di_compile_unit = #llvm.di_compile_unit<id = distinct[0]<>, sourceLanguage = DW_LANG_C, file = #di_file, producer = "MLIR", isOptimized = true, emissionKind = Full>
#di_basic_type = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "uint64_t", sizeInBits = 64, encoding = DW_ATE_unsigned>
-llvm.mlir.global external @global_with_expr1() {addr_space = 0 : i32, dbg_expr = #llvm.di_global_variable_expression<var = <scope = #di_compile_unit, name = "global_with_expr_1", linkageName = "global_with_expr_1", file = #di_file, line = 370, type = #di_basic_type, isLocalToUnit = true, isDefined = true, alignInBits = 8>, expr = <>>} : i64
-llvm.mlir.global external @global_with_expr2() {addr_space = 0 : i32, dbg_expr = #llvm.di_global_variable_expression<var = <scope = #di_compile_unit, name = "global_with_expr_2", linkageName = "global_with_expr_2", file = #di_file, line = 371, type = #di_basic_type, isLocalToUnit = true, isDefined = true, alignInBits = 8>, expr = <[DW_OP_push_object_address, DW_OP_deref]>>} : i64
-llvm.mlir.global external @global_with_expr3() {addr_space = 0 : i32, dbg_expr = #llvm.di_global_variable_expression<var = <scope = #di_compile_unit, name = "global_with_expr_3", linkageName = "global_with_expr_3", file = #di_file, line = 372, type = #di_basic_type, isLocalToUnit = true, isDefined = true, alignInBits = 8>, expr = <[DW_OP_LLVM_arg(0), DW_OP_LLVM_arg(1), DW_OP_plus]>>} : i64
-llvm.mlir.global external @global_with_expr4() {addr_space = 0 : i32, dbg_expr = #llvm.di_global_variable_expression<var = <scope = #di_compile_unit, name = "global_with_expr_4", linkageName = "global_with_expr_4", file = #di_file, line = 373, type = #di_basic_type, isLocalToUnit = true, isDefined = true, alignInBits = 8>, expr = <[DW_OP_LLVM_convert(16, DW_ATE_signed)]>>} : i64
+llvm.mlir.global external @global_with_expr1() {addr_space = 0 : i32, dbg_expr = [#llvm.di_global_variable_expression<var = <scope = #di_compile_unit, name = "global_with_expr_1", linkageName = "global_with_expr_1", file = #di_file, line = 370, type = #di_basic_type, isLocalToUnit = true, isDefined = true, alignInBits = 8>, expr = <>>]} : i64
+llvm.mlir.global external @global_with_expr2() {addr_space = 0 : i32, dbg_expr = [#llvm.di_global_variable_expression<var = <scope = #di_compile_unit, name = "global_with_expr_2", linkageName = "global_with_expr_2", file = #di_file, line = 371, type = #di_basic_type, isLocalToUnit = true, isDefined = true, alignInBits = 8>, expr = <[DW_OP_push_object_address, DW_OP_deref]>>]} : i64
+llvm.mlir.global external @global_with_expr3() {addr_space = 0 : i32, dbg_expr = [#llvm.di_global_variable_expression<var = <scope = #di_compile_unit, name = "global_with_expr_3", linkageName = "global_with_expr_3", file = #di_file, line = 372, type = #di_basic_type, isLocalToUnit = true, isDefined = true, alignInBits = 8>, expr = <[DW_OP_LLVM_arg(0), DW_OP_LLVM_arg(1), DW_OP_plus]>>]} : i64
+llvm.mlir.global external @global_with_expr4() {addr_space = 0 : i32, dbg_expr = [#llvm.di_global_variable_expression<var = <scope = #di_compile_unit, name = "global_with_expr_4", linkageName = "global_with_expr_4", file = #di_file, line = 373, type = #di_basic_type, isLocalToUnit = true, isDefined = true, alignInBits = 8>, expr = <[DW_OP_LLVM_convert(16, DW_ATE_signed)]>>]} : i64
diff --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll
index 09909d7d63b2ab..9ef1942e0787c6 100644
--- a/mlir/test/Target/LLVMIR/Import/debug-info.ll
+++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll
@@ -867,3 +867,27 @@ define void @test() !dbg !3 {
; CHECK: #[[FILE:.+]] = #llvm.di_file<"test.f90" in "">
; CHECK: #[[SP:.+]] = #llvm.di_subprogram<{{.*}}name = "test"{{.*}}>
; CHECK: #llvm.di_common_block<scope = #[[SP]], name = "block", file = #[[FILE]], line = 3>
+
+; // -----
+
+@data = external global i64, !dbg !0, !dbg !5
+
+!llvm.module.flags = !{!8}
+!llvm.dbg.cu = !{!2}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "a", scope: !2, file: !3, line: 2, type: !7)
+!2 = distinct !DICompileUnit(language: DW_LANG_C, file: !3, globals: !4)
+!3 = !DIFile(filename: "test.c", directory: "")
+!4 = !{!0, !5}
+!5 = !DIGlobalVariableExpression(var: !6, expr: !DIExpression())
+!6 = distinct !DIGlobalVariable(name: "b", scope: !2, file: !3, line: 3, type: !7)
+!7 = !DIBasicType(name: "int", size: 32)
+!8 = !{i32 2, !"Debug Info Version", i32 3}
+
+
+; CHECK: #[[VAR1:.+]] = #llvm.di_global_variable<{{.*}}name = "a"{{.*}}>
+; CHECK: #[[VAR2:.+]] = #llvm.di_global_variable<{{.*}}name = "b"{{.*}}>
+; CHECK: #[[EXP1:.+]] = #llvm.di_global_variable_expression<var = #[[VAR1]], expr = <>>
+; CHECK: #[[EXP2:.+]] = #llvm.di_global_variable_expression<var = #[[VAR2]], expr = <>>
+; CHECK: llvm.mlir.global external @data() {{{.*}}dbg_exprs = [#[[EXP1]], #[[EXP2]]]} : i64
diff --git a/mlir/test/Target/LLVMIR/Import/global-variables.ll b/mlir/test/Target/LLVMIR/Import/global-variables.ll
index 879e9135fe4ece..fbeda4cd42af82 100644
--- a/mlir/test/Target/LLVMIR/Import/global-variables.ll
+++ b/mlir/test/Target/LLVMIR/Import/global-variables.ll
@@ -274,8 +274,8 @@ define void @bar() {
; CH...
[truncated]
|
gysit
reviewed
Oct 11, 2024
| @@ -2089,7 +2089,7 @@ void GlobalOp::build(OpBuilder &builder, OperationState &result, Type type, | |||
| Attribute value, uint64_t alignment, unsigned addrSpace, | |||
| bool dsoLocal, bool threadLocal, SymbolRefAttr comdat, | |||
| ArrayRef<NamedAttribute> attrs, | |||
| DIGlobalVariableExpressionAttr dbgExpr) { | |||
| ArrayRef<Attribute> dbgExprs) { | |||
There was a problem hiding this comment.
Suggested change
| ArrayRef<Attribute> dbgExprs) { | |
| ArrayRef<DIGlobalVariableExpressionAttr> dbgExprs) { |
Can this be typed (here and in the other places)?
There was a problem hiding this comment.
I tried this but it did not work. The DIGlobalVariableExpressionArrayAttr becomes an ArrayAttr and that takes an ArrayRef<Attribute> and compiler does not allow ArrayRef<DIGlobalVariableExpressionAttr> to be passed for that. I thought doing manual conversion was not worth the gain.
There was a problem hiding this comment.
Ok makes sense! I think DIGlobalVariableExpressionArrayAttr has a verifier so things should ultimately be checked.
| if (op.getDbgExprs()) { | ||
| for (auto attr : *op.getDbgExprs()) { | ||
| if (auto exprAttr = | ||
| dyn_cast_if_present<DIGlobalVariableExpressionAttr>(attr)) { |
There was a problem hiding this comment.
Can the debug expression array ever have a nullptr entry or contain something else than a DIGlobalVariableExpressionAttr? I would presume not and a cast<DIGlobalVariableExpressionAttr>(attr) makes more range. Maybe you can also use op.getDbgExprs().getAsRange<DIGlobalVariableExpressionAttr>()?
There was a problem hiding this comment.
Thanks for the suggestion. I have replaced it with op.getDbgExprs()->getAsRange<DIGlobalVariableExpressionAttr>().
|
|
||
| // ----- | ||
|
|
||
| // Test multiple DIGlobalVariableExpression on a global |
There was a problem hiding this comment.
Suggested change
| // Test multiple DIGlobalVariableExpression on a global | |
| // Test multiple DIGlobalVariableExpression on a global. |
ultra nit:
There was a problem hiding this comment.
Thanks for catching this. Fixed.
| @@ -1152,6 +1153,9 @@ def LLVM_AddressOfOp : LLVM_Op<"mlir.addressof", | |||
| let hasFolder = 1; | |||
| } | |||
|
|
|||
| def DIGlobalVariableExpressionArrayAttr : | |||
| TypedArrayAttrBase<LLVM_DIGlobalVariableExpressionAttr, "an array of variable expressions">; | |||
There was a problem hiding this comment.
I would consider moving this right below the definition of LLVM_DIGlobalVariableExpressionAttr? We already have such arrays for example for LLVM_AliasScopeArrayAttr.
As I mentioned in the comment above, I was thinking if I could make the argument to the build function more strongly typed. But using |
…file. As suggested in the review comments.
Co-authored-by: Tobias Gysi <tobias.gysi@nextsilicon.com>
I applied the suggested change but it caused a build failure as type of the auto variable was llvm::DIGlobalVariableExpression* and not Attribute. Fixed in this commit. Also removed a stale comment.
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
…lvm#111981) Currently, we allow only one DIGlobalVariableExpressionAttr per global. It is especially evident in import where we pick the first from the list and ignore the rest. In contrast, LLVM allows multiple DIGlobalVariableExpression to be attached to the global. They are needed for correct working of things like DICommonBlock. This PR removes this restriction in mlir. Changes are mostly mechanical. One thing on which I went a bit back and forth was the representation inside GlobalOp. I would be happy to change if there are better ways to do this. --------- Co-authored-by: Tobias Gysi <tobias.gysi@nextsilicon.com>
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 17, 2024
…lvm#111981) Currently, we allow only one DIGlobalVariableExpressionAttr per global. It is especially evident in import where we pick the first from the list and ignore the rest. In contrast, LLVM allows multiple DIGlobalVariableExpression to be attached to the global. They are needed for correct working of things like DICommonBlock. This PR removes this restriction in mlir. Changes are mostly mechanical. One thing on which I went a bit back and forth was the representation inside GlobalOp. I would be happy to change if there are better ways to do this. --------- Co-authored-by: Tobias Gysi <tobias.gysi@nextsilicon.com>
EricWF
pushed a commit
to efcs/llvm-project
that referenced
this pull request
Oct 22, 2024
…lvm#111981) Currently, we allow only one DIGlobalVariableExpressionAttr per global. It is especially evident in import where we pick the first from the list and ignore the rest. In contrast, LLVM allows multiple DIGlobalVariableExpression to be attached to the global. They are needed for correct working of things like DICommonBlock. This PR removes this restriction in mlir. Changes are mostly mechanical. One thing on which I went a bit back and forth was the representation inside GlobalOp. I would be happy to change if there are better ways to do this. --------- Co-authored-by: Tobias Gysi <tobias.gysi@nextsilicon.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, we allow only one DIGlobalVariableExpressionAttr per global. It is especially evident in import where we pick the first from the list and ignore the rest. In contrast, LLVM allows multiple DIGlobalVariableExpression to be attached to the global. They are needed for correct working of things like DICommonBlock. This PR removes this restriction in mlir. Changes are mostly mechanical. One thing on which I went a bit back and forth was the representation inside GlobalOp. I would be happy to change if there are better ways to do this.