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

[NFC] Post review of #1230: Asm igemm nhwc native bf16 kernel on gfx90a #1248

Merged
merged 7 commits into from
Nov 9, 2021

Conversation

shaojiewang
Copy link
Contributor

@shaojiewang shaojiewang commented Oct 29, 2021

Fix post review comment of #1230.
Note: Post review of #1227 will be fixed within #1247 by @carlushuang.


Clarification by @atamazov: This should resolve bullets (2) and (3) from #1241. No Functional Changes.

@codecov

This comment has been minimized.

@junliume
Copy link
Contributor

fix post review comment of #1227 and #1230

@shaojiewang Thanks for the quick actions! Please check the tasks in:
#1241

and if all tasks are completed in this PR, please mark it ready for review :)

@shaojiewang
Copy link
Contributor Author

fix post review comment of #1227 and #1230

@shaojiewang Thanks for the quick actions! Please check the tasks in: #1241

and if all tasks are completed in this PR, please mark it ready for review :)

Yes, sure.

@shaojiewang shaojiewang changed the title [asm igemm][gfx90a]post review of #1227 and #1230 [asm igemm][gfx90a]post review of #1230 Nov 2, 2021
@shaojiewang shaojiewang marked this pull request as ready for review November 3, 2021 02:37
src/include/miopen/solver.hpp Show resolved Hide resolved
src/solver/conv_asm_implicit_gemm_gtc_wrw_nhwc.cpp Outdated Show resolved Hide resolved
src/solver/conv_asm_implicit_gemm_gtc_wrw_nhwc.cpp Outdated Show resolved Hide resolved
@junliume junliume requested a review from atamazov November 4, 2021 00:57
@junliume junliume dismissed atamazov’s stale review November 4, 2021 00:58

Changed made as suggested. CI should consider as passed since last changes are mostly cosmetic.

junliume
junliume previously approved these changes Nov 4, 2021
carlushuang
carlushuang previously approved these changes Nov 4, 2021
Copy link
Contributor

@carlushuang carlushuang left a comment

Choose a reason for hiding this comment

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

LGTM

@shaojiewang shaojiewang dismissed stale reviews from carlushuang and junliume via 5b9293f November 5, 2021 01:41
@shaojiewang
Copy link
Contributor Author

conflicts resolved

Copy link
Contributor

@carlushuang carlushuang left a comment

Choose a reason for hiding this comment

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

LGTM

@junliume
Copy link
Contributor

junliume commented Nov 7, 2021

@atamazov gentle ping for review input on this one :)

Copy link
Contributor

@atamazov atamazov left a comment

Choose a reason for hiding this comment

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

@atamazov atamazov changed the title [asm igemm][gfx90a]post review of #1230 [NFC] Post review of #1230 Nov 8, 2021
@shaojiewang
Copy link
Contributor Author

let's move to #1230 (comment)

Copy link
Contributor

@atamazov atamazov left a comment

Choose a reason for hiding this comment

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

LGTM!

@junliume junliume changed the title [NFC] Post review of #1230 [NFC] Post review of #1230: Asm igemm nhwc native bf16 kernel on gfx90a Nov 9, 2021
@junliume junliume merged commit 37c635e into develop Nov 9, 2021
@junliume junliume deleted the asm_igemm_gfx90a_altfp16_bf16_post_review_branch branch January 7, 2022 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants