[MLIR] Improve translation of DISubrange. #93689
Conversation
The DISubrange can take integer, dwarf expressions or variables. The current translation only handled integers. This PR adds handling of dwarf expressions and variables.
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-llvm Author: Abid Qadeer (abidh) ChangesThe DISubrange can take integer, dwarf expressions or variables. The current translation only handled integers. This PR adds handling of dwarf expressions and variables. Full diff: https://github.com/llvm/llvm-project/pull/93689.diff 5 Files Affected:
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 535cf8dfd2ced..87fa6b388f7a4 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -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,
+ OptionalParameter<"::mlir::Attribute">:$lowerBound,
+ OptionalParameter<"::mlir::Attribute">:$upperBound,
+ OptionalParameter<"::mlir::Attribute">:$stride
);
let assemblyFormat = "`<` struct(params) `>`";
}
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
index 6c011b3c756ff..0010589c5068d 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
@@ -217,21 +217,34 @@ 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) -> mlir::Attribute {
+ if (data.isNull())
+ return nullptr;
+
+ if (auto *constInt = llvm::dyn_cast<llvm::ConstantInt *>(data)) {
return IntegerAttr::get(IntegerType::get(context, 64),
constInt->getSExtValue());
- return IntegerAttr();
+ } else if (auto *expr = llvm::dyn_cast<llvm::DIExpression *>(data)) {
+ return translateExpression(expr);
+ } else if (auto *var = llvm::dyn_cast<llvm::DIVariable *>(data)) {
+ if (auto *local = llvm::dyn_cast<llvm::DILocalVariable>(var))
+ return translate(local);
+ else if (auto *global = llvm::dyn_cast<llvm::DIGlobalVariable>(var))
+ return translate(global);
+ llvm_unreachable("Unknown variable type");
+ }
+ llvm_unreachable("Unknown DISubrange::BoundType");
};
- IntegerAttr count = getIntegerAttrOrNull(node->getCount());
- IntegerAttr upperBound = getIntegerAttrOrNull(node->getUpperBound());
+ auto count = getAttrOrNull(node->getCount());
+ auto 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
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
index dfb7d4952157d..d619cf43fde86 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -314,11 +314,24 @@ 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()));
+
+ if (auto intAttr = llvm::dyn_cast<IntegerAttr>(attr)) {
+ return llvm::ConstantAsMetadata::get(llvm::ConstantInt::getSigned(
+ llvm::Type::getInt64Ty(llvmCtx), intAttr.getInt()));
+ } else if (auto exprAttr = llvm::dyn_cast<LLVM::DIExpressionAttr>(attr)) {
+ return translateExpression(exprAttr);
+ } else if (auto localVar =
+ llvm::dyn_cast<LLVM::DILocalVariableAttr>(attr)) {
+ return translate(localVar);
+ } else if (auto globalVar =
+ llvm::dyn_cast<LLVM::DIGlobalVariableAttr>(attr)) {
+ return translate(globalVar);
+ } else {
+ llvm_unreachable("Unexpected Attribute value");
+ }
};
return llvm::DISubrange::get(llvmCtx, getMetadataOrNull(attr.getCount()),
getMetadataOrNull(attr.getLowerBound()),
diff --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll
index 72c828448d088..0c6a7368b7b88 100644
--- a/mlir/test/Target/LLVMIR/Import/debug-info.ll
+++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll
@@ -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
@@ -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)
@@ -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)
+
; // -----
diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index e5c233bc5e9bb..1913a5564d929 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -519,3 +519,41 @@ 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.
+
+#di_basic_type = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "int">
+#di_file = #llvm.di_file<"debug-info.ll" in "/">
+#di_compile_unit = #llvm.di_compile_unit<id = distinct[1]<>, sourceLanguage = DW_LANG_Fortran95, file = #di_file, isOptimized = false, emissionKind = Full>
+#di_composite_type = #llvm.di_composite_type<tag = DW_TAG_array_type, name = "expr_elements", baseType = #di_basic_type, flags = Vector, elements = #llvm.di_subrange<count = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(16), DW_OP_deref]>>>
+#di_subroutine_type = #llvm.di_subroutine_type<types = #di_basic_type, #di_composite_type>
+#di_subprogram = #llvm.di_subprogram<id = distinct[2]<>, compileUnit = #di_compile_unit, scope = #di_file, name = "subranges", file = #di_file, subprogramFlags = Definition, type = #di_subroutine_type>
+#di_local_variable = #llvm.di_local_variable<scope = #di_subprogram, name = "size">
+#gv1 = #llvm.di_global_variable<scope = #di_compile_unit, name = "gv", file = #di_file, line = 3, type = #di_basic_type>
+#gve1 = #llvm.di_global_variable_expression<var = #gv1, expr = <>>
+#di_composite_type1 = #llvm.di_composite_type<tag = DW_TAG_array_type, name = "var_elements", baseType = #di_basic_type, flags = Vector, elements = #llvm.di_subrange<count = #di_local_variable, stride = #gv1>>
+#di_local_variable1 = #llvm.di_local_variable<scope = #di_subprogram, name = "size", type = #di_composite_type1>
+#loc1 = loc("test.f90": 1:1)
+#loc2 = loc(fused<#di_subprogram>[#loc1])
+
+module attributes {} {
+ llvm.mlir.global external @gv() {dbg_expr = #gve1} : i64
+
+ llvm.func @subranges(%arg: !llvm.ptr) {
+ llvm.intr.dbg.declare #di_local_variable1 = %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: ![[SR2:[0-9]+]], stride: ![[GV:[0-9]+]])
+// CHECK: ![[SR2]] = !DILocalVariable(name: "size"{{.*}})
|
| if (data.isNull()) | ||
| return nullptr; | ||
|
|
||
| if (auto *constInt = llvm::dyn_cast<llvm::ConstantInt *>(data)) { |
There was a problem hiding this comment.
Nit: Can you use a TypeSwitch here?
There was a problem hiding this comment.
Thanks for the review. Please see the comments below why I could not use TypeSwitch here..
| } else if (auto *var = llvm::dyn_cast<llvm::DIVariable *>(data)) { | ||
| if (auto *local = llvm::dyn_cast<llvm::DILocalVariable>(var)) | ||
| return translate(local); | ||
| else if (auto *global = llvm::dyn_cast<llvm::DIGlobalVariable>(var)) | ||
| return translate(global); | ||
| llvm_unreachable("Unknown variable type"); | ||
| } |
There was a problem hiding this comment.
Is there a reason to not directly cast to a local or global variable?
There was a problem hiding this comment.
The data is PointerUnion having DIVariable as one of its members. Direct casting to DILocalVariable or DIGlobalVariable does not work. For this reason, I could not use TypeSwitch here as the DIVariable case returns 2 different types which causes build to fail. I have changed the if/else to if as all had returns.
| return translate(global); | ||
| llvm_unreachable("Unknown variable type"); | ||
| } | ||
| llvm_unreachable("Unknown DISubrange::BoundType"); |
There was a problem hiding this comment.
Given my experience with debug metadata, I'm very afraid of seeing llvm_unreachable anywhere here.
There was a problem hiding this comment.
I have changed it to return nullptr. I was trying to guard against any changes in things that DISubrange can accept but I think that is quite unlikely.
| auto count = getAttrOrNull(node->getCount()); | ||
| auto upperBound = getAttrOrNull(node->getUpperBound()); |
There was a problem hiding this comment.
Nit: Only use auto when the type is mentioned on the RHS or too complex to write out (iterators, ranges, template mess, etc.)
| OptionalParameter<"IntegerAttr">:$lowerBound, | ||
| OptionalParameter<"IntegerAttr">:$upperBound, | ||
| OptionalParameter<"IntegerAttr">:$stride | ||
| OptionalParameter<"::mlir::Attribute">:$count, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@gysit do you know if there is any way to achieve this?
There was a problem hiding this comment.
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.
| llvm::dyn_cast<LLVM::DIGlobalVariableAttr>(attr)) { | ||
| return translate(globalVar); | ||
| } else { | ||
| llvm_unreachable("Unexpected Attribute value"); |
There was a problem hiding this comment.
This is not verified on the input IR, so it's technically legal to break this assumption. Therefore, you should not use llvm_unreachable here.
There was a problem hiding this comment.
I changed it to return nullptr.
| return llvm::ConstantAsMetadata::get(llvm::ConstantInt::getSigned( | ||
| llvm::Type::getInt64Ty(llvmCtx), attr.getInt())); | ||
|
|
||
| if (auto intAttr = llvm::dyn_cast<IntegerAttr>(attr)) { |
There was a problem hiding this comment.
Nit: Use a TypeSwitch here.
Side note: When you would keep this, you should turn all the else ifs into normal ifs, given that all of them contain a return.
| #di_basic_type = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "int"> | ||
| #di_file = #llvm.di_file<"debug-info.ll" in "/"> | ||
| #di_compile_unit = #llvm.di_compile_unit<id = distinct[1]<>, sourceLanguage = DW_LANG_Fortran95, file = #di_file, isOptimized = false, emissionKind = Full> | ||
| #di_composite_type = #llvm.di_composite_type<tag = DW_TAG_array_type, name = "expr_elements", baseType = #di_basic_type, flags = Vector, elements = #llvm.di_subrange<count = #llvm.di_expression<[DW_OP_push_object_address, DW_OP_plus_uconst(16), DW_OP_deref]>>> | ||
| #di_subroutine_type = #llvm.di_subroutine_type<types = #di_basic_type, #di_composite_type> | ||
| #di_subprogram = #llvm.di_subprogram<id = distinct[2]<>, compileUnit = #di_compile_unit, scope = #di_file, name = "subranges", file = #di_file, subprogramFlags = Definition, type = #di_subroutine_type> | ||
| #di_local_variable = #llvm.di_local_variable<scope = #di_subprogram, name = "size"> | ||
| #gv1 = #llvm.di_global_variable<scope = #di_compile_unit, name = "gv", file = #di_file, line = 3, type = #di_basic_type> | ||
| #gve1 = #llvm.di_global_variable_expression<var = #gv1, expr = <>> | ||
| #di_composite_type1 = #llvm.di_composite_type<tag = DW_TAG_array_type, name = "var_elements", baseType = #di_basic_type, flags = Vector, elements = #llvm.di_subrange<count = #di_local_variable, stride = #gv1>> | ||
| #di_local_variable1 = #llvm.di_local_variable<scope = #di_subprogram, name = "size", type = #di_composite_type1> | ||
| #loc1 = loc("test.f90": 1:1) | ||
| #loc2 = loc(fused<#di_subprogram>[#loc1]) |
There was a problem hiding this comment.
Are all of these attribute parameters required?
Also, can you try to split the attributes over different lines (as for the first test in this file), to ensure this is somewhat in the 80 lines character limit (which is not properly enforced in this file).
There was a problem hiding this comment.
Testcase is bigger because we need to test with the local and global variables in the type apart from expression. I have modified the test to respect 80 character limit.
1. Handle formatting nits. 2. Use TypeSwitch instead of multiple if/else 3. Respect 80 column limit in tests.
| OptionalParameter<"IntegerAttr">:$lowerBound, | ||
| OptionalParameter<"IntegerAttr">:$upperBound, | ||
| OptionalParameter<"IntegerAttr">:$stride | ||
| OptionalParameter<"::mlir::Attribute">:$count, |
There was a problem hiding this comment.
@gysit do you know if there is any way to achieve this?
| auto getIntegerAttrOrNull = [&](llvm::DISubrange::BoundType data) { | ||
| if (auto *constInt = llvm::dyn_cast_or_null<llvm::ConstantInt *>(data)) | ||
| auto getAttrOrNull = | ||
| [&](llvm::DISubrange::BoundType data) -> mlir::Attribute { |
There was a problem hiding this comment.
| [&](llvm::DISubrange::BoundType data) -> mlir::Attribute { | |
| [&](llvm::DISubrange::BoundType data) -> Attribute { |
Nit: The mlir:: namespace is not required in most of MLIR's cpp files.
There was a problem hiding this comment.
Thanks for catching this. Fixed now.
| [&](llvm::DISubrange::BoundType data) -> mlir::Attribute { | ||
| if (data.isNull()) | ||
| return nullptr; | ||
| if (auto *constInt = llvm::dyn_cast<llvm::ConstantInt *>(data)) |
There was a problem hiding this comment.
| if (auto *constInt = llvm::dyn_cast<llvm::ConstantInt *>(data)) | |
| if (auto *constInt = dyn_cast<llvm::ConstantInt *>(data)) |
Nit: dyn_cast is reexported in the mlir namespace, so it is not necessary to qualify this.
The same applies to all other casts in this PR.
|
|
||
| llvm::Metadata *metadata = | ||
| llvm::TypeSwitch<Attribute, llvm::Metadata *>(attr) | ||
| .Case<IntegerAttr>([&](IntegerAttr intAttr) { |
There was a problem hiding this comment.
| .Case<IntegerAttr>([&](IntegerAttr intAttr) { | |
| .Case([&](IntegerAttr intAttr) { |
Nit: The template parameter is inferred from the type of the lambda, so it can be omitted here and in the other cases.
There was a problem hiding this comment.
Thanks for the tip. I have fixed it.
| #loc1 = loc("test.f90": 1:1) | ||
| #loc2 = loc(fused<#sp>[#loc1]) | ||
|
|
||
| module attributes {} { |
There was a problem hiding this comment.
Nit: Is it necessary to wrap this into a module? Remove it when it's not required.
I don't have a great idea either. I think going with an attribute is probably best we can do for now. |
|
|
||
| // CHECK: !DICompositeType(tag: DW_TAG_array_type, name: "var_elements"{{.*}}elements: ![[ELEMENTS2:[0-9]+]]) | ||
| // CHECK: ![[ELEMENTS2]] = !{![[ELEMENT2:[0-9]+]]} | ||
| // CHECK: ![[ELEMENT2]] = !DISubrange(count: ![[SR2:[0-9]+]], stride: ![[GV:[0-9]+]]) |
There was a problem hiding this comment.
| // CHECK: ![[ELEMENT2]] = !DISubrange(count: ![[SR2:[0-9]+]], stride: ![[GV:[0-9]+]]) | |
| // CHECK: ![[ELEMENT2]] = !DISubrange(count: ![[LV:[0-9]+]], stride: ![[GV:[0-9]+]]) |
nit: maybe LV for local variable?
| mlir::Attribute count = getAttrOrNull(node->getCount()); | ||
| mlir::Attribute upperBound = getAttrOrNull(node->getUpperBound()); |
There was a problem hiding this comment.
| mlir::Attribute count = getAttrOrNull(node->getCount()); | |
| mlir::Attribute upperBound = getAttrOrNull(node->getUpperBound()); | |
| Attribute count = getAttrOrNull(node->getCount()); | |
| Attribute upperBound = getAttrOrNull(node->getUpperBound()); |
nit: the mlir namespace is not necessary.
| OptionalParameter<"IntegerAttr">:$lowerBound, | ||
| OptionalParameter<"IntegerAttr">:$upperBound, | ||
| OptionalParameter<"IntegerAttr">:$stride | ||
| OptionalParameter<"::mlir::Attribute">:$count, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
LGTM
I am not sure if the suggestion of using a PointerUnion got lost? I think that may actually be a good solution to improve type safety of the bounds type info and it is also in line with what LLVM does. Or did you try and it did also get to cumbersome to use?
I spent time on that suggestion today but I have not been able to make it work till now. Build error suggest that |
Thanks for tying this! You are right this probably needs a custom parser/printer a bit similar to |
The DISubrange can take integer, dwarf expressions or variables. The current translation only handled integers. This PR adds handling of dwarf expressions and variables.