[LLVM] Expand tvm::Type to DWARF conversion#14568
Conversation
|
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.
Generated by tvm-bot |
Prior to this commit, DWARF debug symbols for `float32`, `int8`, and `int32` could be generated, with other datatypes resulting in an error. This commit expands the range of types with debug symbols to allow for any `DLDataTypeCode::kDLInt`, `kDLUint`, or `kDLFloat`, regardless of the bitsize.
Lunderberg
force-pushed
the
llvm_additional_dwarf_types
branch
from
98b294b to
ee7ac4b
Compare
echuraev
left a comment
echuraev
left a comment
There was a problem hiding this comment.
Is it necessary to add a unit test? What do you think?
| std::stringstream ss; | ||
| ss << name; | ||
|
|
||
| if (!dtype.is_bool()) { | ||
| ss << dtype.bits(); | ||
| } | ||
| if (dtype.lanes() > 1) { | ||
| ss << "x" << dtype.lanes(); | ||
| } |
There was a problem hiding this comment.
There is already a function that gets the string from DataType here.
Please add #include <sstream> for std::stringstream if you decide to keep it.
There was a problem hiding this comment.
Thank you, and updated to use DLDataType2String.
|
|
||
| } else if (auto* prim_type = ty_tir.as<PrimTypeNode>()) { | ||
| DataType dtype = prim_type->dtype; | ||
| auto [name, dwarf_type] = [&]() -> std::tuple<const char*, llvm::dwarf::TypeKind> { |
There was a problem hiding this comment.
llvm::StringRef is the "LLVM way" rather than const char*.
There was a problem hiding this comment.
Thank you. With the DLDataType2String it ended up being unnecessary, but I will keep that in mind for the future.
|
@echuraev Good point. Probably not strictly required, as the existing DWARF handling isn't tested, but this does impact what function signatures are allowed when not using the PackedFunc API. I've added a unit test that fails on main, and passes with this change. |
kparzysz-quic
left a comment
kparzysz-quic
left a comment
There was a problem hiding this comment.
Your comment says that you've added a unit test for this, but it's not in this PR. Is it included in another PR? In any case, feel free to merge this now if you want it in sooner than later, and commit the testcase in another PR.
|
@kparzysz-quic Thank you, and thank you for the catch! Looks like I added the unit test in my local branch, then forgot to push. |
4 participants
Prior to this commit, DWARF debug symbols for
float32,int8, andint32could be generated, with other datatypes resulting in an error. This commit expands the range of types with debug symbols to allow for anyDLDataTypeCode::kDLInt,kDLUint, orkDLFloat, regardless of the bitsize.