Skip to content

benchdnn: graph: support non-contiguous tensor validation for matmul and strides rewriting #3448

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jul 4, 2025
Merged

benchdnn: graph: support non-contiguous tensor validation for matmul and strides rewriting #3448

merged 6 commits into from
Jul 4, 2025

Conversation

wzt1997
Copy link
Contributor

@wzt1997 wzt1997 commented Jun 19, 2025 •
edited
Loading

General

Resolve MFDNN-10785, to support non-contiguous memory validation for packed QKV.

  • support validation for strides that cannot be represented in tags: in this case, the graph driver will respect the strides.
  • support strides rewriting through --in-shapes knob
  • change tag rewriting: --in-shapes=0:abcd to --in-shapes=0:*abcd
  • add more test cases with packed qkv
  • correct some existing illegal strides.

@wzt1997 wzt1997 self-assigned this Jun 19, 2025
@github-actions github-actions bot added documentation A request to change/fix/improve the documentation. Codeowner: @oneapi-src/onednn-doc component:graph-api Codeowner: @oneapi-src/onednn-graph component:tests Codeowner: @oneapi-src/onednn-arch labels Jun 19, 2025
@wzt1997 wzt1997 force-pushed the zhitao/support-non-dense-mem-validation branch 2 times, most recently from b47507b to 1c9e405 Compare June 20, 2025 01:49
@wzt1997 wzt1997 marked this pull request as ready for review June 20, 2025 01:51
@wzt1997 wzt1997 requested review from a team as code owners June 20, 2025 01:51
dzarukin
dzarukin reviewed Jun 22, 2025
View reviewed changes
tests/benchdnn/doc/driver_graph.md Outdated
stride of a graph input tensor that includes `ID` in a graph with `SHAPE` and
`TAG` values. `SHAPE` and `TAG` are separated by a `*`. Multiple
inputs may be specified using the `+` delimiter.
- `--in-shapes=ID:SHAPE[*STRIDE/TAG+ID:SHAPE*STRIDE/TAG+...]` -- Override a
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `--in-shapes=ID:SHAPE[*STRIDE/TAG+ID:SHAPE*STRIDE/TAG+...]` -- Override a
- `--in-shapes=ID:SHAPE[*STRIDES][+ID:SHAPE*STRIDES+...]` -- Override a

STRIDE/TAG might be considered as a syntax. Suggest to use STRIDES uniformly and carefully explain in the doc that STRIDES can be represented as a tag abcd which implies one, two, three, and used as a convenience shortcut to the alternative, full specification method via tensor with a similar look to shapes which implies one, two, three.

Additionally, a note that an asterisk * is mandatory to allow modification of full specified strides without shape modification.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. The document is updated.

tests/benchdnn/graph/flex_rewrite.cpp Outdated
@@ -902,8 +902,10 @@ void flex_rewrite_t::infer_output_shape(

for (auto &lt : aop.in_lts_) {
lt.shape_ = gi[lt.id_];
lt.stride_
= memory_tag2strides(gi[lt.id_], dgraph.lt_2_mtag_[lt.id_]);
if (dgraph.lt_2_mtag_[lt.id_] != "not_available") {
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when it is "not_available"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The logic is completed now:

  • for graph input ports, no action is needed as the shape and strides have been filled during the input ports handling
  • for the intermediate results, they will use dense format or tags inherited from the inputs, so we should not meet not_available tags,

ElaineBao
ElaineBao reviewed Jun 23, 2025
View reviewed changes
@wzt1997 wzt1997 force-pushed the zhitao/support-non-dense-mem-validation branch 2 times, most recently from d3aaa45 to 5c021dc Compare June 23, 2025 07:47
rongzha1
rongzha1 reviewed Jun 24, 2025
View reviewed changes
@wzt1997
Copy link
Contributor Author

wzt1997 commented Jun 27, 2025

make test
set test_scope=NIGHTLY
disable benchdnn_all
enable benchdnn_graph

dzarukin
dzarukin approved these changes Jun 30, 2025
View reviewed changes
@wzt1997 wzt1997 force-pushed the zhitao/support-non-dense-mem-validation branch from 5c021dc to 71e9ec0 Compare July 1, 2025 08:50
@wzt1997 wzt1997 force-pushed the zhitao/support-non-dense-mem-validation branch from 71e9ec0 to 6264e1d Compare July 2, 2025 01:37
@wzt1997
Copy link
Contributor Author

wzt1997 commented Jul 2, 2025

make test
set test_scope=NIGHTLY
disable benchdnn_all
enable benchdnn_graph

ranukund
ranukund reviewed Jul 4, 2025
View reviewed changes
Copy link
Contributor

@ranukund ranukund left a comment

Choose a reason for hiding this comment

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

Some edits suggested, please incorporate as you see fit! Thanks

@wzt1997 wzt1997 force-pushed the zhitao/support-non-dense-mem-validation branch from 6264e1d to 08c1dfb Compare July 4, 2025 08:22
@wzt1997
Copy link
Contributor Author

wzt1997 commented Jul 4, 2025

Some edits suggested, please incorporate as you see fit! Thanks

Thanks a lot! Your suggestions have been incorporated. Please take another look.

ranukund
ranukund approved these changes Jul 4, 2025
View reviewed changes
Copy link
Contributor

@ranukund ranukund left a comment

Choose a reason for hiding this comment

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

The .md file looks good, thanks!

@TaoLv TaoLv merged commit 33b04dd into main Jul 4, 2025
23 checks passed
@TaoLv TaoLv deleted the zhitao/support-non-dense-mem-validation branch July 4, 2025 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ElaineBao ElaineBao

@rongzha1 rongzha1

@dzarukin dzarukin

@ranukund ranukund

Assignees

@wzt1997 wzt1997

Labels
component:graph-api Codeowner: @oneapi-src/onednn-graph component:tests Codeowner: @oneapi-src/onednn-arch documentation A request to change/fix/improve the documentation. Codeowner: @oneapi-src/onednn-doc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@wzt1997 @ElaineBao @rongzha1 @dzarukin @ranukund @TaoLv