-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Target] Replace utility functions with target.features #12455
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
Conversation
341389d to
523f186
Compare
523f186 to
dcc1ddd
Compare
dcc1ddd to
df0ad05
Compare
Following on from apache#12454 this patch removes the utility functions in favour of the centralised `target.features` property.
df0ad05 to
b3dfa48
Compare
This removes many references to `is_aarch64` in favour of `is_asimd` for which it was often a proxy. Also relaxed a Compute Library test as the schedules are now different with proper arch detection
6b7629b to
960a519
Compare
678f54c to
6df4400
Compare
|
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 |
lhutton1
left a comment
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.
Looks good, apart from one small thing!
6df4400 to
2870b0d
Compare
|
@lhutton1 fixed the typo, PTAL 😸 |
ce573d3 to
4561d40
Compare
lhutton1
left a comment
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.
LGTM!
|
Thanks @Mousius! |
Following on from apache#12454 this patch removes the utility functions in favour of the centralised `target.features` property.
Following on from apache#12454 this patch removes the utility functions in favour of the centralised `target.features` property.
32-bit targets apache#12455 slightly altered the behaviour when selecting an int8 conv2d schedule. Previously conditions that decide which schedule to select used `is_aarch64` which checks for the existance of `aarch64` in the target triple. However, the conditions now use `has_asimd` which is true if `aarch64` exists in the target triple OR `+neon` is used in the mattr. Both `conv2d_NHWC_quantized_interleaved.arm_cpu` and `depthwise_conv2d_nhwc.arm_cpu` makes calls to LLVM intrinsics that require both `aarch64` and `+neon`. But in the case of the target `rasp4b`, the updated conditions result in compilation failure since the target has `+neon` but doesn't have `aarch64` in the target triple. The conditions have been updated to fix the compilation failure. Likewise, the previous behaviour of the condition for `conv2d_nhwc_spatial_pack.arm_cpu` has been restored ensure a program with a 32-bit target can still be compiled. Finally, we should only select the `depthwise_conv2d_nhwc_dsp.arm_cpu` schedule when a backend that understands `pragma_import_c` has been selected, i.e. "c". For a more detailed discussion of the issue please see: https://discuss.tvm.apache.org/t/tflite-llvm-llvm-error-when-compiling-tflite-model/15411 Change-Id: Idcf541ecdb7fee7d392bfbe5bd1f7cb478408938
…-bit targets (#15468) [Relay][Strategy] Fix `arm_cpu` int8 conv2d schedule selection for 32-bit targets #12455 slightly altered the behaviour when selecting an int8 conv2d schedule. Previously conditions that decide which schedule to select used `is_aarch64` which checks for the existance of `aarch64` in the target triple. However, the conditions now use `has_asimd` which is true if `aarch64` exists in the target triple OR `+neon` is used in the mattr. Both `conv2d_NHWC_quantized_interleaved.arm_cpu` and `depthwise_conv2d_nhwc.arm_cpu` makes calls to LLVM intrinsics that require both `aarch64` and `+neon`. But in the case of the target `rasp4b`, the updated conditions result in compilation failure since the target has `+neon` but doesn't have `aarch64` in the target triple. The conditions have been updated to fix the compilation failure. Likewise, the previous behaviour of the condition for `conv2d_nhwc_spatial_pack.arm_cpu` has been restored ensure a program with a 32-bit target can still be compiled. Finally, we should only select the `depthwise_conv2d_nhwc_dsp.arm_cpu` schedule when a backend that understands `pragma_import_c` has been selected, i.e. "c". For a more detailed discussion of the issue please see: https://discuss.tvm.apache.org/t/tflite-llvm-llvm-error-when-compiling-tflite-model/15411
Following on from #12454 this patch removes the utility functions in favour of the centralised
target.featuresproperty.