Skip to content
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

BatchedMatMul: MKL gemm_batch support #1181

Merged
merged 2 commits into from Mar 16, 2023
Merged

BatchedMatMul: MKL gemm_batch support #1181

merged 2 commits into from Mar 16, 2023

Conversation

lukastruemper
Copy link
Contributor

No description provided.

@lukastruemper lukastruemper marked this pull request as draft December 17, 2022 14:37
@lukastruemper lukastruemper marked this pull request as ready for review December 17, 2022 16:49
MKL_INT ldb_array[group_count] = {{ {ldb} }};
MKL_INT ldc_array[group_count] = {{ {ldc} }};

const {dtype}** A = new const {dtype}*[{BATCH}];
Copy link
Collaborator

Choose a reason for hiding this comment

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

why malloc in the middle of a potentially performance-critical code??

Copy link
Collaborator

Choose a reason for hiding this comment

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

btw that's the reason why we didn't implement it thus far.

@tbennun
Copy link
Collaborator

tbennun commented Dec 18, 2022

you can try adding the allocation to the init code (if it depends on input symbols or is constant) or the stack (if the batch size is known and small)

@lukastruemper
Copy link
Contributor Author

you can try adding the allocation to the init code (if it depends on input symbols or is constant) or the stack (if the batch size is known and small)

I don't know how to move this part to the init part. For BERT with batch size 6-8, the old implementation is much slower than this version

@tbennun
Copy link
Collaborator

tbennun commented Mar 16, 2023

I'll approve for now, but this should probably use some inflatable workspace.

@tbennun tbennun enabled auto-merge March 16, 2023 16:29
@tbennun tbennun merged commit dd69131 into master Mar 16, 2023
9 checks passed
@lukastruemper lukastruemper deleted the usrs/lukas/bmm branch March 25, 2023 14:50
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.

None yet

2 participants