-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[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.
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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()); | ||||||||||
mlir::Attribute count = getAttrOrNull(node->getCount()); | ||||||||||
mlir::Attribute upperBound = getAttrOrNull(node->getUpperBound()); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
nit: the mlir namespace is not necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||||||||||
// 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 | ||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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: ![[SR2:[0-9]+]], stride: ![[GV:[0-9]+]]) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
nit: maybe LV for local variable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||||||
// CHECK: ![[SR2]] = !DILocalVariable(name: "size"{{.*}}) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 theDISubrangeAttr
in place ofIntegerAttr
. But it seems a bit overkill to me. Also made the usage a bit more cumbersome.There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.