tvm
tvm copied to clipboard
[Array] Add deduction guide for Array(IterType,IterType) constructor
Previously, the Array(IterType,IterType) constructor required the class template parameter T to be explicitly provided. This commit adds a template deduction guide, specifying the intended type T as the type produced by de-referencing the iterator, to remove the requirement.
Fixes https://github.com/apache/tvm/issues/14600
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.
- No users to tag found in teams:
arraySee #10317 for details
Generated by tvm-bot
@Lunderberg @Hzfengsy Thanks you for your advice and patch. However, I'm sorry to tell you that this crash still exists after using the patch mentioned in this PR. The build crash message:
/tvm/src/tir/analysis/identify_memcpy.cc:298:27: error: no viable constructor or deduction guide for deduction of template arguments of 'Array'
output->push_back(Array{ptr->source, ptr->dest});
@jikechao Could you please try to upgrade your cxx compiler?
@Hzfengsy I have updated llvm-gcc-12.0 which is the latest version that is supported by Ubuntu16.04. But this bug exists.
As a workaround, I modify the source code by replacing Array{ptr->source, ptr->dest} withArray<ObjectRef>{ptr->source, ptr->dest} to avoid the build failure.
Interesting. While the change compiles locally, it looks like the CI is having issues with this patch, with the older version of gcc not interpreting the deduction guide correctly.
Looking at this specific use case again, it looks like it is also dependent on the ambiguity between Array{first_element, second_element} and Array{iter_begin, iter_end}. The deduction guides would help handle the case where the second usage has ambiguous type, but it wouldn't handle the case where the first usage is intended. We may be able to remove this ambiguity by moving the static_assert into a SFINAE parameter, so that the initializer_list case is the only remaining overload.
Can you try making the following change in include/tvm/runtime/container/array.h?
// Before
template <typename IterType>
Array(IterType first, IterType last) {
static_assert(is_valid_iterator_v<T, IterType>,
"IterType cannot be inserted into a tvm::Array<T>");
Assign(first, last);
}
// After
template <typename IterType, typename = std::enable_if_t<is_valid_iterator_v<T, IterType>>>
Array(IterType first, IterType last) {
Assign(first, last);
}
That said, if this is the only instance of this issue, it probably is easier to change from Array{...} to Array<ObjectRef>{...}.
@Lunderberg Thanks for your efforts. Indeed, only one instance of this issue.
After changing Array{...} to Array<ObjectRef>{...}. and changing the source code you provided above, build sucessfully.
@tvm-bot rerun
Trying to clear out some of my (very) old PRs. This PR is now rebased onto main including the fix in this comment. If it passes CI, then it's ready to merge.