-
Notifications
You must be signed in to change notification settings - Fork 159
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
[onert-micro] Add svdf cmsis-nn #13719
[onert-micro] Add svdf cmsis-nn #13719
Conversation
d2bb71f
to
729f163
Compare
nnas_include(OptionTools) | ||
|
||
envoption(EXTERNAL_DOWNLOAD_SERVER "https://github.com") | ||
envoption(CMSIS_NN_6_0_0_URL ${EXTERNAL_DOWNLOAD_SERVER}/ARM-software/CMSIS-NN/archive/refs/tags/v6.0.0.tar.gz) |
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.
@BalyshevArtem this PR does not work with CMSIS_NN 4.1 ? I am not sure which CMSIS_NN version in TizenRT.
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.
Hmmm, if it is critical, I can rewrite it with 4.1 cmsis-nn, but current implementation required 6.0. TFLITE-micro uses the 6.0 version. Rewrite using 4.0?
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.
It does not matter if it is compatible to TizenRT(but I do not know which version in TizenRT in use. It 'was' compatible to 4.1)
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.
FYI, internal TizenRT use the same file at https://github.com/Samsung/TizenRT/blob/master/external/include/cmsis_nn/arm_nnfunctions.h#L1675 for cmsis_nn. Seem like it is based on 4.1 instead of 6.0 (based on parameter numbers)
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.
Rewrite using 4.0?
Is it possible ? I do not know about SVDF. But CMSIS_NN 4.1 does not use kernel_sum
at all in SVDF kernel.
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.
Is it possible ?
Yes :)
Okay, I will rewrite to 4.1 and close prs with cmsis-nn version updating
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.
Done, ready for review
This draft adds svdf cmsis-nn kernel. ONE-DCO-1.0-Signed-off-by: Artem Balyshev <[email protected]
729f163
to
c434966
Compare
This pr adds svdf cmsis-nn kernel.
for #13634
ONE-DCO-1.0-Signed-off-by: Artem Balyshev [email protected]