Use xa_nnlib for svdf for Fusion F1. #47098
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for contributing to TensorFlow Lite Micro. To keep this process moving along, we'd like to make sure that you have completed the items on this list:
We would like to have a discussion on the Github issue first to determine the best path forward, and then proceed to the PR review. |
|
tagging @pnikam-cad @nyadla-sys @bhanuprakashbv |
advaitjain
added a commit
to advaitjain/tensorflow
that referenced
this pull request
Feb 12, 2021
Confirmed that the test passes with: ``` make -f tensorflow/lite/micro/tools/make/Makefile TARGET=xtensa OPTIMIZED_KERNEL_DIR=xtensa TARGET_ARCH=fusion_f1 XTENSA_CORE=F1_190305_swupgrade test_kernel_softmax_test -j8 ``` However, the latency improvement is only ~1000 ticks, as tested with: ``` make -f tensorflow/lite/micro/tools/make/Makefile TARGET=xtensa OPTIMIZED_KERNEL_DIR=xtensa TARGET_ARCH=fusion_f1 XTENSA_CORE=F1_190305_swupgrade test_keyword_benchmark -j8 ``` Since Softmax is currently a small fraction of the overall keyword_benchmark latency we will focus on the latency of only this particular OP. With the optimized implementation: ``` SOFTMAX took 749 ticks (0 ms). ``` Reference implementation: ``` SOFTMAX took 2052 ticks (2 ms). ``` And with the LUT hifimini implementation (for completeness): ``` SOFTMAX took 1142 ticks (1 ms). ``` The gain of ~1500 ticks ticks is still worth merging because after all the optimizations (e.g. tensorflow#47098), this will still mean a ~5% improvement for the keyword benchmark. And the benefits might be more significant for other models too.
njeffrie
approved these changes
Feb 12, 2021
| (data.effective_scale_1_a), data.effective_scale_1_b), | ||
| 0); | ||
| } | ||
|
|
There was a problem hiding this comment.
This is no longer part of the feature matmul... perhaps leave a comment here to mark the start of the time weights dot product + activation stage.
The code in this change is the subset of functionality needed for int8 svdf for Hifi4 copied from https://github.com/pnikam-cad/tensorflow/blob/a737c1e3945bc70022259479ad24133a343ec906/tensorflow/lite/micro/kernels/xtensa_hifi/svdf.cc Note that the current change has not pulled in either the floating point implementation or the Hifi5 implementation. Profiled the keryword_benchmark with the following command: ``` make -f tensorflow/lite/micro/tools/make/Makefile TARGET=xtensa OPTIMIZED_KERNEL_DIR=xtensa TARGET_ARCH=fusion_f1 XTENSA_CORE=F1_190305_swupgrade run_keyword_benchmark -j8 ``` gives a latency of 38516 ticks with this change vs 152642 ticks without this change. Per OP latency with this change: ``` KeywordRunNIerations(1) took 38516 ticks (38 ms) QUANTIZE took 3758 ticks (3 ms). SVDF took 4753 ticks (4 ms). FULLY_CONNECTED took 1353 ticks (1 ms). SVDF took 4211 ticks (4 ms). FULLY_CONNECTED took 1353 ticks (1 ms). SVDF took 3145 ticks (3 ms). FULLY_CONNECTED took 1353 ticks (1 ms). SVDF took 4211 ticks (4 ms). FULLY_CONNECTED took 1353 ticks (1 ms). SVDF took 2890 ticks (2 ms). SVDF took 3583 ticks (3 ms). SVDF took 3054 ticks (3 ms). FULLY_CONNECTED took 1091 ticks (1 ms). SOFTMAX took 2042 ticks (2 ms). QUANTIZE took 366 ticks (0 ms). ``` Without this change: ``` KeywordRunNIerations(1) took 152642 ticks (152 ms) QUANTIZE took 3758 ticks (3 ms). SVDF took 38003 ticks (38 ms). FULLY_CONNECTED took 1353 ticks (1 ms). SVDF took 18803 ticks (18 ms). FULLY_CONNECTED took 1353 ticks (1 ms). SVDF took 18803 ticks (18 ms). FULLY_CONNECTED took 1353 ticks (1 ms). SVDF took 18803 ticks (18 ms). FULLY_CONNECTED took 1353 ticks (1 ms). SVDF took 13907 ticks (13 ms). SVDF took 15827 ticks (15 ms). SVDF took 15827 ticks (15 ms). FULLY_CONNECTED took 1091 ticks (1 ms). SOFTMAX took 2042 ticks (2 ms). QUANTIZE took 366 ticks (0 ms). ``` Also confirmed that the kernel_svdf_test passes with: ``` make -f tensorflow/lite/micro/tools/make/Makefile TARGET=xtensa OPTIMIZED_KERNEL_DIR=xtensa TARGET_ARCH=fusion_f1 XTENSA_CORE=F1_190305_swupgrade test_kernel_svdf_test -j8 ```
advaitjain
commented
Feb 12, 2021
advaitjain
force-pushed
the
fusion-f1-svdf
branch
from
de53e77
to
55633bf
Compare
advaitjain
added
kokoro:force-run
advaitjain
added
ready to pull
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The code in this change is the subset of functionality needed for int8 svdf for Hifi4 copied from https://github.com/pnikam-cad/tensorflow/blob/a737c1e3945bc70022259479ad24133a343ec906/tensorflow/lite/micro/kernels/xtensa_hifi/svdf.cc
Note that the current change has not pulled in either the floating point implementation or the Hifi5 implementation.
Profiled the keryword_benchmark with the following command:
gives a latency of 38516 ticks with this change vs 152642 ticks without this change.
Per OP latency with this change:
Without this change:
Also confirmed that the kernel_svdf_test passes with:
Progress towards http://b/177457688