Skip to content

Conversation

lhutton1
Copy link
Contributor

@lhutton1 lhutton1 commented Aug 3, 2023

#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

cc @Mousius @ashutosh-arm @ekalda @neildhickey

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
@tvm-bot
Copy link
Collaborator

tvm-bot commented Aug 3, 2023

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

@github-actions github-actions bot requested review from Mousius, asparkhi and ekalda August 3, 2023 14:09
Copy link
Contributor

@ekalda ekalda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lhutton1, well spotted!

@ekalda ekalda merged commit 63a95d5 into apache:main Aug 4, 2023
@ekalda
Copy link
Contributor

ekalda commented Aug 4, 2023

Thanks @lhutton1!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants