[SVE] Add support for scalable data type strings #16612
Conversation
This commit adds support for representing scalable vectors using the string data type format. For example, "float32xvscalex4" may be used to represent the following scalable type: `DataType(kDLFloat, 32, /*lanes=*/4, /*is_scalable=*/true)`. Co-authored-by: Elen Kalda <[email protected]> Co-authored-by: Neil Hickey <[email protected]> Change-Id: I883dbdf29f500cc6bf6f9dc3097fa1d5af12b020
|
@tvm-bot rerun |
| } | ||
| /*! \return whether type is a scalar type. */ | ||
| bool is_scalar() const { return lanes() == 1; } | ||
| bool is_scalar() const { return !is_scalable_vector() && lanes() == 1; } |
There was a problem hiding this comment.
I think it would make more sense to use !is_scalable_or_fixed_length_vector() here. The current version is going to throw an error if applied on a scalable vector.
There was a problem hiding this comment.
Due to placing a call to is_scalable_vector() before lanes(), is_scalar() should work with scalable vectors in its current form, I'll add some tests for this. I don't think we can simply return !is_scalable_or_fixed_length_vector() as it wouldn't account for the case when lanes == 0.
There was a problem hiding this comment.
Ok yes, thinking about it, this probably is the most solid way to do it, even though it reads a bit awkward 👍
Change-Id: I3e6c56a0f1e171278c50fefe10bddd1feeca1b69
|
Thanks @lhutton1! |
|
there is a regression due to concurrent merge, cc @Lunderberg |
|
thanks for raising @tqchen, I'm taking a look now |
A default value for lanes was unintentionally removed in apache#16612, this PR fixes this which in turn fixes the test failure seen in `test_tensor_dtype_lanes` in CI. Change-Id: If1b06cb836d62e85620971fd1ad0498c9fdc66b9
|
Apologies for missing this previously, I've uploaded #16649 which I'd expect to fix the failing test case |
|
Any idea why the CI didn't catch this? |
|
CI was last run on the patch yesterday (26th Feb) while the PR that caught the issue was merged a week ago #16563. I didn't rebase before pushing, but had expected CI to do this automatically. I think it might have something to do with a missing call to |
A default value for lanes was unintentionally removed in #16612, this PR fixes this which in turn fixes the test failure seen in `test_tensor_dtype_lanes` in CI.
This commit adds support for representing scalable vectors using the string data type format. For example, "float32xvscalex4" may be used to represent the following scalable type: `DataType(kDLFloat, 32, /*lanes=*/4, /*is_scalable=*/true)`. --------- Co-authored-by: Elen Kalda <[email protected]> Co-authored-by: Neil Hickey <[email protected]>
A default value for lanes was unintentionally removed in apache#16612, this PR fixes this which in turn fixes the test failure seen in `test_tensor_dtype_lanes` in CI.
3 participants
This commit adds support for representing scalable vectors using the string data type format. For example, "float32xvscalex4" may be used to represent the following scalable type:
DataType(kDLFloat, 32, /*lanes=*/4, /*is_scalable=*/true).Co-authored-by: Elen Kalda [email protected]
Co-authored-by: Neil Hickey [email protected]