Skip to content

Conversation

@NonerKao
Copy link

No description provided.

@NonerKao
Copy link
Author

@oliIMG @ken-unger If you don't mind, I believe we should drop #7116 since this PR is aligned better to current upstream status.

@ken-unger
Copy link
Contributor

@oliIMG @ken-unger If you don't mind, I believe we should drop #7116 since this PR is aligned better to current upstream status.

No issues from me @NonerKao. I'm not sure if @oliIMG is contributing here any longer and as you say #7116 is stale now. I'm doing likewise with some of the stale Q* rvv PRs. Good to have your new PR and I'm happy to take a look at the changes, but I'm not a maintainer and so can't do any approvals.

@NonerKao
Copy link
Author

Hi @dsharlet , would you mind taking a look when you get a chance?

@NonerKao NonerKao force-pushed the dev-alan-contribute-spmm branch from c313deb to 0d03661 Compare March 31, 2025 02:51
The baseline implementation of SPMM kernel was mostly inherited PR google#7116.

This patch futher simplifies the leftover handling, aligns the template
and naming to other rvv kernels, and also contains miscellaneous fixes
according to recent refactor.
@NonerKao NonerKao force-pushed the dev-alan-contribute-spmm branch from 0d03661 to 7de15ab Compare March 31, 2025 02:55
@NonerKao
Copy link
Author

NonerKao commented Mar 31, 2025

@dsharlet Sorry for the inconvenience, but here are some updates:

  • Rebase and fix conflicts due to recent code reformat
  • Add copyright headers
  • Fix the test generation scripts for dwconv2d-nchw kernels

All tests for these kernels can be generated. Also they pass: 39 for f32-spmm, 56 for f32-conv-hwc2chw, and 314 for f32-dwconv2d-chw, with --benchmark_filter=rvv.

@NonerKao
Copy link
Author

NonerKao commented Apr 8, 2025

Hi @dsharlet

Any concerns on this PR?

Copy link
Collaborator

@dsharlet dsharlet left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. In the future, we'd prefer to see changes like this broken up into smaller changes, this looks like it should be at least 3 separate PRs (conv, dwconv, spmm).

@ken-unger
Copy link
Contributor

@dsharlet we'll need your help to merge this PR. Thank you.

copybara-service bot pushed a commit that referenced this pull request Apr 10, 2025
--
60d5e0b by Gary Yi-Hung Chen <[email protected]>:

Add RVV f32 SPMM kernel

The baseline implementation of SPMM kernel was mostly inherited PR #7116.

This patch futher simplifies the leftover handling, aligns the template
and naming to other rvv kernels, and also contains miscellaneous fixes
according to recent refactor.

--
f7e7eba by Gary Yi-Hung Chen <[email protected]>:

Add RVV f32-conv-hwc2chw-3x3s2p1c3 kernel

--
7de15ab by Gary Yi-Hung Chen <[email protected]>:

Add RVV f32-dwconv2d-chw kernel

FUTURE_COPYBARA_INTEGRATE_REVIEW=#8081 from NonerKao:dev-alan-contribute-spmm 7de15ab
PiperOrigin-RevId: 745899104
@NonerKao
Copy link
Author

NonerKao commented Apr 11, 2025

Hi @dsharlet and @ken-unger Thanks for pushing this patch further. Since the test reveals a failed case, are we supposed to fix that fail?

FAIL: //test:qs8_rdsum_minmax_fp32_test (Exit 1) (see /home/runner/.cache/bazel/_bazel_runner/fee536d8bef76115a69b6bcd195c06ca/execroot/_main/bazel-out/k8-fastbuild/testlogs/test/qs8_rdsum_minmax_fp32_test/test.log)

copybara-service bot pushed a commit that referenced this pull request Apr 14, 2025
--
60d5e0b by Gary Yi-Hung Chen <[email protected]>:

Add RVV f32 SPMM kernel

The baseline implementation of SPMM kernel was mostly inherited PR #7116.

This patch futher simplifies the leftover handling, aligns the template
and naming to other rvv kernels, and also contains miscellaneous fixes
according to recent refactor.

--
f7e7eba by Gary Yi-Hung Chen <[email protected]>:

Add RVV f32-conv-hwc2chw-3x3s2p1c3 kernel

--
7de15ab by Gary Yi-Hung Chen <[email protected]>:

Add RVV f32-dwconv2d-chw kernel

FUTURE_COPYBARA_INTEGRATE_REVIEW=#8081 from NonerKao:dev-alan-contribute-spmm 7de15ab
PiperOrigin-RevId: 745899104
@copybara-service copybara-service bot merged commit e74cc77 into google:master Apr 14, 2025
21 checks passed
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.

4 participants