VMulCAddC-Replaced yaml files with header table #7411
+371
−3,934
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
nitheshsrikanth-mcw
force-pushed
the
xnn_vmulcadd
branch
2 times, most recently
from
8bfc637
to
47192e6
Compare
dsharlet
reviewed
Nov 4, 2024
src/f16-vmulcaddc/f16-vmulcaddc.h
Outdated
|
|
||
| #ifndef XNN_UKERNEL | ||
| #define XNN_UKERNEL(arch_flags, ukernel, row_tile,channel_tile, datatype) \ | ||
| XNN_UKERNEL_WITH_PARAMS(arch_flags, ukernel,row_tile,channel_tile , datatype, void, /*init_params=*/nullptr) |
There was a problem hiding this comment.
Commas should immediately follow prior value, and then space after comma:
XNN_UKERNEL_WITH_PARAMS(arch_flags, ukernel,row_tile, channel_tile, datatype, void, /*init_params=*/nullptr)
Please fix other instances in this file too (many above and below).
nitheshsrikanth-mcw
force-pushed
the
xnn_vmulcadd
branch
5 times, most recently
from
9669c60
to
d8dd82d
Compare
dsharletg
reviewed
Nov 6, 2024
tools/generate-vmulcaddc-test.py
Outdated
There was a problem hiding this comment.
Same comments as the other PRs
nitheshsrikanth-mcw
force-pushed
the
xnn_vmulcadd
branch
from
d29eeb2
to
c799529
Compare
nitheshsrikanth-mcw
force-pushed
the
xnn_vmulcadd
branch
5 times, most recently
from
ac65c37
to
cf7f1c3
Compare
dsharletg
approved these changes
Nov 8, 2024
nitheshsrikanth-mcw
force-pushed
the
xnn_vmulcadd
branch
from
1f6456a
to
757569f
Compare
copybara-service bot
pushed a commit
that referenced
this pull request
Nov 12, 2024
-- 47192e6 by nithesh <[email protected]>: VMulCAddC-Replaced yaml files with header table Replaced yaml files with header table for vmulcadd op -- d8dd82d by nithesh <[email protected]>: Addressed review comments -- cf7f1c3 by nithesh <[email protected]>: Addressed review comments FUTURE_COPYBARA_INTEGRATE_REVIEW=#7411 from nitheshsrikanth-mcw:xnn_vmulcadd 1f6456a PiperOrigin-RevId: 695565047
copybara-service bot
pushed a commit
that referenced
this pull request
Nov 12, 2024
-- 47192e6 by nithesh <[email protected]>: VMulCAddC-Replaced yaml files with header table Replaced yaml files with header table for vmulcadd op -- d8dd82d by nithesh <[email protected]>: Addressed review comments -- cf7f1c3 by nithesh <[email protected]>: Addressed review comments FUTURE_COPYBARA_INTEGRATE_REVIEW=#7411 from nitheshsrikanth-mcw:xnn_vmulcadd 1f6456a PiperOrigin-RevId: 695565047
copybara-service bot
pushed a commit
that referenced
this pull request
Nov 12, 2024
-- 47192e6 by nithesh <[email protected]>: VMulCAddC-Replaced yaml files with header table Replaced yaml files with header table for vmulcadd op -- d8dd82d by nithesh <[email protected]>: Addressed review comments -- cf7f1c3 by nithesh <[email protected]>: Addressed review comments FUTURE_COPYBARA_INTEGRATE_REVIEW=#7411 from nitheshsrikanth-mcw:xnn_vmulcadd 1f6456a PiperOrigin-RevId: 695565047
|
Update kernel declarations? |
|
@dsharlet Thank you for your comment. I did attempt to update the kernel declarations as suggested, but I encountered an issue during the process that I wasn't able to resolve today. I will revisit the task tomorrow and work on resolving it then. |
Replaced yaml files with header table for vmulcadd op
nitheshsrikanth-mcw
force-pushed
the
xnn_vmulcadd
branch
from
c06ce2e
to
a68dfeb
Compare
nitheshsrikanth-mcw
force-pushed
the
xnn_vmulcadd
branch
5 times, most recently
from
24e9a1d
to
6897480
Compare
nitheshsrikanth-mcw
force-pushed
the
xnn_vmulcadd
branch
from
6897480
to
5021e63
Compare
|
@dsharlet i have updated the kernel declarations. Thanks! |
dsharletg
reviewed
Nov 21, 2024
| #define XNN_TEST_VMULCADDC_ROW_DIV(ukernel, arch_flags, row_tile, channels_tile, datatype, params_type, init_params) \ | ||
| TEST(ukernel, ROW_div) \ | ||
| { \ | ||
| for (size_t rows = row_tile * 2; rows <= row_tile * 4; rows += row_tile) { \ |
There was a problem hiding this comment.
TEST_REQUIRES_ARCH_FLAGS(arch_flags)? Does this pass tests locally for you?
There was a problem hiding this comment.
Thanks for pointing out the missing TEST_REQUIRES_ARCH_FLAGS(arch_flags). The issue with the illegal instruction for AVX-512 has now been resolved and now the tests are passing.
nitheshsrikanth-mcw
force-pushed
the
xnn_vmulcadd
branch
from
32a9c6c
to
e3de96d
Compare
dsharlet
reviewed
Nov 22, 2024
| XNN_INTERNAL void fn_name( \ | ||
| size_t m, \ | ||
| size_t c, \ | ||
| const xnn_float16* x, \ | ||
| const xnn_float16* x, \ |
There was a problem hiding this comment.
We should be able to use datatype here, and #include both headers with the one XNN_UKERNEL definition.
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.
No description provided.