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.

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

[HOTFIX] WORKAROUND_ISSUE_2038: Disable FP16 and BF16 in ConvHipImplicitGemmV4R1Fwd for MI100/200 and all new targets. #2041

Closed
wants to merge 5 commits into from
Closed

[HOTFIX] WORKAROUND_ISSUE_2038: Disable FP16 and BF16 in ConvHipImplicitGemmV4R1Fwd for MI100/200 and all new targets. #2041

wants to merge 5 commits into from

Conversation

atamazov
Copy link
Contributor

@atamazov atamazov commented Mar 23, 2023 •
edited
Loading

JehandadKhan
JehandadKhan previously approved these changes Mar 23, 2023
View reviewed changes
@junliume

This comment was marked as off-topic.

Sign in to view
@atamazov atamazov mentioned this pull request Mar 23, 2023
Open
@atamazov
Copy link
Contributor Author

@atamazov this issue seems to be able to workaround by 277e35c and it is caused by hipRTC header file warning messages are treated as error when -DBUILD_DEV=On.

Your observations do not match to what I see the the CI log of #2033:

[2023-03-23T18:53:48.490Z]  73/323 Test #128: smoke_solver_ConvHipImplicitGemmV4R1 
...
[2023-03-23T18:53:48.490Z] MIOpen(HIP): Warning [GetAllConfigs] ConvHipImplicitGemmV4R1Fwd: Searching the best solution among 33...
[2023-03-23T18:53:48.490Z] MIOpen(HIP): Warning [Monitor] 0/0/5 0.23334, best within recent 1: 0.23334 #0 16,32,8,2,2,2,4,4,2,4,8,1,16,1,4,32, ETA:0 sec.
[2023-03-23T18:53:48.490Z] MIOpen(HIP): Warning [Monitor] 1/0/5 0.23334, best within recent 1: 0.750709 #1 16,32,4,2,4,4,2,4,2,4,4,1,16,1,4,16, ETA:53.7877 sec.
[2023-03-23T18:53:48.490Z] MIOpen(HIP): Warning [Monitor] 2/0/5 0.23334, best within recent 1: 0.285917 #2 16,128,4,2,4,4,4,4,4,4,4,2,16,2,2,128, ETA:28.3322 sec.
[2023-03-23T18:53:48.490Z] MIOpen(HIP): Warning [Monitor] 3/0/5 0.114943, best within recent 1: 0.114943 #3 16,128,16,2,4,4,4,4,4,4,16,1,16,1,2,128, ETA:15.7516 sec.
[2023-03-23T18:53:48.490Z] MIOpen(HIP): Warning [GenericSearch] Done: 5/0/5, best #3 0.114943 16,128,16,2,4,4,4,4,4,4,16,1,16,1,2,128
[2023-03-23T18:53:48.490Z] MIOpen(HIP): Warning [GenericSearch] ...Score: 2.44989 (default time 0.281597)
[2023-03-23T18:53:48.490Z] FAILED: 0.535381
[2023-03-23T18:53:48.490Z] Max diff: 255
[2023-03-23T18:53:48.490Z] Mismatch at 0: 30 != 129

@atamazov atamazov mentioned this pull request Mar 23, 2023
Merged
junliume
junliume previously approved these changes Mar 23, 2023
View reviewed changes
junliume
junliume reviewed Mar 24, 2023
View reviewed changes
test/CMakeLists.txt Outdated Show resolved Hide resolved
@junliume junliume dismissed stale reviews from JehandadKhan and themself via 9ee84e0 March 24, 2023 05:06
@junliume junliume changed the title [HOTFIX] WORKAROUND_ISSUE_2038: Disable BF16 in ConvHipImplicitGemmV4R1Fwd for MI100/200 and all new targets. [HOTFIX] WORKAROUND_ISSUE_2038: Disable BF16/FP16 in ConvHipImplicitGemmV4R1Fwd for MI100/200 and all new targets. Mar 24, 2023
@atamazov
Copy link
Contributor Author

@junliume @JehandadKhan @carlushuang @asroy PR is ready for the next round of reviewing and CI testing.

atamazov
atamazov commented Mar 24, 2023
View reviewed changes
Copy link
Contributor Author

@atamazov atamazov left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@junliume Thanks for 9ee84e0. It disables testing of HALF totally, but we still need to keep it for non-MI100/200 targets. Fixed now.

@atamazov atamazov changed the title [HOTFIX] WORKAROUND_ISSUE_2038: Disable BF16/FP16 in ConvHipImplicitGemmV4R1Fwd for MI100/200 and all new targets. [HOTFIX] WORKAROUND_ISSUE_2038: Disable FP16 and BF16 in ConvHipImplicitGemmV4R1Fwd for MI100/200 and all new targets. Mar 24, 2023
@atamazov atamazov marked this pull request as draft March 26, 2023 22:45
@atamazov
Copy link
Contributor Author

Converted to draft due to #2043 (comment)

@atamazov
Copy link
Contributor Author

Seems not necessary, but let's keep this PR and branch for a month or so.

@atamazov atamazov closed this Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@junliume junliume junliume left review comments

@JehandadKhan JehandadKhan JehandadKhan left review comments

@BrianHarrisonAMD BrianHarrisonAMD Awaiting requested review from BrianHarrisonAMD BrianHarrisonAMD will be requested when the pull request is marked ready for review BrianHarrisonAMD is a code owner

@BradPepersAMD BradPepersAMD Awaiting requested review from BradPepersAMD BradPepersAMD will be requested when the pull request is marked ready for review BradPepersAMD is a code owner

Assignees
No one assigned
Labels
correctness ON_HOLD TESTING_CI_PASSED urgency_blocker workaround
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@atamazov @junliume @JehandadKhan