Skip to content

[MLIR] Improve translation of DISubrange. #93689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 4, 2024
Merged

[MLIR] Improve translation of DISubrange. #93689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cfa6b5e
[MLIR] Improve translation of DISubrange.
abidh May 28, 2024
c6f1cdf
Handle review comments.
abidh May 30, 2024
55d747d
Handle review comments.
abidh May 31, 2024
417e85c
Handle review comments.
abidh Jun 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,10 +625,10 @@ def LLVM_DINamespaceAttr : LLVM_Attr<"DINamespace", "di_namespace",
def LLVM_DISubrangeAttr : LLVM_Attr<"DISubrange", "di_subrange", /*traits=*/[],
"DINodeAttr"> {
let parameters = (ins
OptionalParameter<"IntegerAttr">:$count,
OptionalParameter<"IntegerAttr">:$lowerBound,
OptionalParameter<"IntegerAttr">:$upperBound,
OptionalParameter<"IntegerAttr">:$stride
OptionalParameter<"::mlir::Attribute">:$count,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not recall this, but I guess attribute parameters cannot be constraint through table gen, or can they? Ideally, we would explicitly list which types are legal here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not find a simple way to add that constraint so I am open to suggestions here. My initial approach was to add a DISubrangeValueAttr type which has fields for all the types that we expect (e.g. IntegerAttr, DIExpressionAttr, etc). Then use that in the DISubrangeAttr in place of IntegerAttr. But it seems a bit overkill to me. Also made the usage a bit more cumbersome.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gysit do you know if there is any way to achieve this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LLVM BoundType is a pointer union. That could be an option that maybe less cumbersome than a separate attribute. Worst case we can also go for a plain attribute.

OptionalParameter<"::mlir::Attribute">:$lowerBound,
OptionalParameter<"::mlir::Attribute">:$upperBound,
OptionalParameter<"::mlir::Attribute">:$stride
);
let assemblyFormat = "`<` struct(params) `>`";
}
Expand Down
27 changes: 19 additions & 8 deletions mlir/lib/Target/LLVMIR/DebugImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,21 +217,32 @@ DISubprogramAttr DebugImporter::translateImpl(llvm::DISubprogram *node) {
}

DISubrangeAttr DebugImporter::translateImpl(llvm::DISubrange *node) {
auto getIntegerAttrOrNull = [&](llvm::DISubrange::BoundType data) {
if (auto *constInt = llvm::dyn_cast_or_null<llvm::ConstantInt *>(data))
auto getAttrOrNull = [&](llvm::DISubrange::BoundType data) -> Attribute {
if (data.isNull())
return nullptr;
if (auto *constInt = dyn_cast<llvm::ConstantInt *>(data))
return IntegerAttr::get(IntegerType::get(context, 64),
constInt->getSExtValue());
return IntegerAttr();
if (auto *expr = dyn_cast<llvm::DIExpression *>(data))
return translateExpression(expr);
if (auto *var = dyn_cast<llvm::DIVariable *>(data)) {
if (auto *local = dyn_cast<llvm::DILocalVariable>(var))
return translate(local);
if (auto *global = dyn_cast<llvm::DIGlobalVariable>(var))
return translate(global);
return nullptr;
}
return nullptr;
};
IntegerAttr count = getIntegerAttrOrNull(node->getCount());
IntegerAttr upperBound = getIntegerAttrOrNull(node->getUpperBound());
Attribute count = getAttrOrNull(node->getCount());
Attribute upperBound = getAttrOrNull(node->getUpperBound());
// Either count or the upper bound needs to be present. Otherwise, the
// metadata is invalid. The conversion might fail due to unsupported DI nodes.
if (!count && !upperBound)
return {};
return DISubrangeAttr::get(
context, count, getIntegerAttrOrNull(node->getLowerBound()), upperBound,
getIntegerAttrOrNull(node->getStride()));
return DISubrangeAttr::get(context, count,
getAttrOrNull(node->getLowerBound()), upperBound,
getAttrOrNull(node->getStride()));
}

DISubroutineTypeAttr
Expand Down
22 changes: 19 additions & 3 deletions mlir/lib/Target/LLVMIR/DebugTranslation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,27 @@ llvm::DINamespace *DebugTranslation::translateImpl(DINamespaceAttr attr) {
}

llvm::DISubrange *DebugTranslation::translateImpl(DISubrangeAttr attr) {
auto getMetadataOrNull = [&](IntegerAttr attr) -> llvm::Metadata * {
auto getMetadataOrNull = [&](Attribute attr) -> llvm::Metadata * {
if (!attr)
return nullptr;
return llvm::ConstantAsMetadata::get(llvm::ConstantInt::getSigned(
llvm::Type::getInt64Ty(llvmCtx), attr.getInt()));

llvm::Metadata *metadata =
llvm::TypeSwitch<Attribute, llvm::Metadata *>(attr)
.Case([&](IntegerAttr intAttr) {
return llvm::ConstantAsMetadata::get(llvm::ConstantInt::getSigned(
llvm::Type::getInt64Ty(llvmCtx), intAttr.getInt()));
})
.Case([&](LLVM::DIExpressionAttr expr) {
return translateExpression(expr);
})
.Case([&](LLVM::DILocalVariableAttr local) {
return translate(local);
})
.Case<>([&](LLVM::DIGlobalVariableAttr global) {
return translate(global);
})
.Default([&](Attribute attr) { return nullptr; });
return metadata;
};
return llvm::DISubrange::get(llvmCtx, getMetadataOrNull(attr.getCount()),
getMetadataOrNull(attr.getLowerBound()),
Expand Down
23 changes: 16 additions & 7 deletions mlir/test/Target/LLVMIR/Import/debug-info.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,17 @@ define void @derived_type() !dbg !3 {

; CHECK-DAG: #[[INT:.+]] = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "int">
; CHECK-DAG: #[[FILE:.+]] = #llvm.di_file<"debug-info.ll" in "/">
; CHECK-DAG: #[[VAR:.+]] = #llvm.di_local_variable<{{.*}}name = "size">
; CHECK-DAG: #[[GV:.+]] = #llvm.di_global_variable<{{.*}}name = "gv"{{.*}}>
; CHECK-DAG: #[[COMP1:.+]] = #llvm.di_composite_type<tag = DW_TAG_array_type, name = "array1", line = 10, baseType = #[[INT]], sizeInBits = 128, alignInBits = 32>
; CHECK-DAG: #[[COMP2:.+]] = #llvm.di_composite_type<{{.*}}, file = #[[FILE]], scope = #[[FILE]], baseType = #[[INT]]>
; CHECK-DAG: #[[COMP3:.+]] = #llvm.di_composite_type<{{.*}}, flags = Vector, elements = #llvm.di_subrange<count = 4 : i64>>
; CHECK-DAG: #[[COMP4:.+]] = #llvm.di_composite_type<{{.*}}, flags = Vector, elements = #llvm.di_subrange<lowerBound = 0 : i64, upperBound = 4 : i64, stride = 1 : i64>>
; CHECK-DAG: #[[COMP5:.+]] = #llvm.di_composite_type<{{.*}}, flags = Vector>
; CHECK-DAG: #llvm.di_subroutine_type<types = #[[COMP1]], #[[COMP2]], #[[COMP3]], #[[COMP4]], #[[COMP5]]>
; CHECK-DAG: #[[COMP5:.+]] = #llvm.di_composite_type<{{.*}}, name = "var_elements"{{.*}}elements = #llvm.di_subrange<count = #[[VAR]], stride = #[[GV]]>>
; CHECK-DAG: #[[COMP6:.+]] = #llvm.di_composite_type<{{.*}}, name = "expr_elements"{{.*}}elements = #llvm.di_subrange<count = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(16), DW_OP_deref]>>>
; CHECK-DAG: #llvm.di_subroutine_type<types = #[[COMP1]], #[[COMP2]], #[[COMP3]], #[[COMP4]], #[[COMP5]], #[[COMP6]]>

@gv = external global i64

define void @composite_type() !dbg !3 {
ret void
Expand All @@ -179,7 +184,7 @@ define void @composite_type() !dbg !3 {
!2 = !DIFile(filename: "debug-info.ll", directory: "/")
!3 = distinct !DISubprogram(name: "composite_type", scope: !2, file: !2, spFlags: DISPFlagDefinition, unit: !1, type: !4)
!4 = !DISubroutineType(types: !5)
!5 = !{!7, !8, !9, !10, !18}
!5 = !{!7, !8, !9, !10, !18, !22}
!6 = !DIBasicType(name: "int")
!7 = !DICompositeType(tag: DW_TAG_array_type, name: "array1", line: 10, size: 128, align: 32, baseType: !6)
!8 = !DICompositeType(tag: DW_TAG_array_type, name: "array2", file: !2, scope: !2, baseType: !6)
Expand All @@ -189,12 +194,16 @@ define void @composite_type() !dbg !3 {
!12 = !DISubrange(lowerBound: 0, upperBound: 4, stride: 1)
!13 = !{!11}
!14 = !{!12}

; Verifies that unsupported subrange nodes are skipped.
!15 = !DISubrange(count: !16)
!15 = !DISubrange(count: !16, stride: !23)
!16 = !DILocalVariable(scope: !3, name: "size")
!17 = !{!15}
!18 = !DICompositeType(tag: DW_TAG_array_type, name: "unsupported_elements", flags: DIFlagVector, elements: !17, baseType: !6)
!18 = !DICompositeType(tag: DW_TAG_array_type, name: "var_elements", flags: DIFlagVector, elements: !17, baseType: !6)
!19 = !DISubrange(count: !20)
!20 = !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 16, DW_OP_deref)
!21 = !{!19}
!22 = !DICompositeType(tag: DW_TAG_array_type, name: "expr_elements", flags: DIFlagVector, elements: !21, baseType: !6)
!23 = !DIGlobalVariable(name: "gv", scope: !1, file: !2, line: 3, type: !6, isLocal: false, isDefinition: false)


; // -----

Expand Down
45 changes: 45 additions & 0 deletions mlir/test/Target/LLVMIR/llvmir-debug.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,3 +519,48 @@ llvm.func @fn_with_composite() {
// CHECK-SAME: associated: !DIExpression(DW_OP_lit0, DW_OP_eq)
// CHECK-SAME: allocated: !DIExpression(DW_OP_lit0, DW_OP_ne)
// CHECK-SAME: rank: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 16, DW_OP_deref)

// -----

// Test that Subrange works with expression and variables.

#bt = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "int">
#file = #llvm.di_file<"debug-info.ll" in "/">
#cu = #llvm.di_compile_unit<id = distinct[1]<>,
sourceLanguage = DW_LANG_Fortran95, file = #file, isOptimized = false,
emissionKind = Full>
#comp_ty1 = #llvm.di_composite_type<tag = DW_TAG_array_type,
name = "expr_elements", baseType = #bt, flags = Vector,
elements = #llvm.di_subrange<count = #llvm.di_expression<
[DW_OP_push_object_address, DW_OP_plus_uconst(16), DW_OP_deref]>>>
#srty = #llvm.di_subroutine_type<types = #bt, #comp_ty1>
#sp = #llvm.di_subprogram<compileUnit = #cu, scope = #file, name = "subranges",
file = #file, subprogramFlags = Definition, type = #srty>
#lvar = #llvm.di_local_variable<scope = #sp, name = "size">
#gv = #llvm.di_global_variable<scope = #cu, name = "gv", file = #file,
line = 3, type = #bt>
#gve = #llvm.di_global_variable_expression<var = #gv, expr = <>>
#comp_ty2 = #llvm.di_composite_type<tag = DW_TAG_array_type,
name = "var_elements", baseType = #bt, flags = Vector,
elements = #llvm.di_subrange<count = #lvar, stride = #gv>>
#lvar2 = #llvm.di_local_variable<scope = #sp, name = "var", type = #comp_ty2>
#loc1 = loc("test.f90": 1:1)
#loc2 = loc(fused<#sp>[#loc1])

llvm.mlir.global external @gv() {dbg_expr = #gve} : i64

llvm.func @subranges(%arg: !llvm.ptr) {
llvm.intr.dbg.declare #lvar2 = %arg : !llvm.ptr
llvm.return
} loc(#loc2)

// CHECK-LABEL: define void @subranges
// CHECK: ![[GV:[0-9]+]] = {{.*}}!DIGlobalVariable(name: "gv"{{.*}})
// CHECK: !DICompositeType(tag: DW_TAG_array_type, name: "expr_elements"{{.*}}elements: ![[ELEMENTS1:[0-9]+]])
// CHECK: ![[ELEMENTS1]] = !{![[ELEMENT1:[0-9]+]]}
// CHECK: ![[ELEMENT1]] = !DISubrange(count: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 16, DW_OP_deref))

// CHECK: !DICompositeType(tag: DW_TAG_array_type, name: "var_elements"{{.*}}elements: ![[ELEMENTS2:[0-9]+]])
// CHECK: ![[ELEMENTS2]] = !{![[ELEMENT2:[0-9]+]]}
// CHECK: ![[ELEMENT2]] = !DISubrange(count: ![[LV:[0-9]+]], stride: ![[GV:[0-9]+]])
// CHECK: ![[LV]] = !DILocalVariable(name: "size"{{.*}})
Loading