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

Fix GPU_LAUNCH_KERNEL for hip-clang #445

Closed
wants to merge 1 commit into from
Closed

Fix GPU_LAUNCH_KERNEL for hip-clang #445

wants to merge 1 commit into from

Conversation

yxsamliu
Copy link

@yxsamliu yxsamliu commented May 8, 2019

No description provided.

@ROCm-Apps-Test
Copy link

Can one of the admins verify this patch?

3 similar comments
@ROCm-Apps-Test
Copy link

Can one of the admins verify this patch?

@ROCm-Apps-Test
Copy link

Can one of the admins verify this patch?

@ROCm-Apps-Test
Copy link

Can one of the admins verify this patch?

@yxsamliu yxsamliu requested a review from whchung May 8, 2019 02:29
@whchung
Copy link
Collaborator

whchung commented May 8, 2019

@yxsamliu to cope with changes upstream, we are in the processing of getting #439 merged, which abolishes GPU_LAUNCH_KERNEL and consolidate on a new syntax congruent with upstream TF_CHECK_OK(GpuLaunchKernel(.

@whchung
Copy link
Collaborator

whchung commented May 8, 2019

... so instead of continue changing call sites of GPU_LAUNCH_KERNEL, could you check if #439 works with hip-clang?

whchung
whchung requested changes May 8, 2019
View reviewed changes
Copy link
Collaborator

@whchung whchung left a comment

Choose a reason for hiding this comment

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

The PR is unlikely be taken due to incoming #439. Please help check if #439 works on hip-clang toolchain.

@yxsamliu
Copy link
Author

yxsamliu commented May 8, 2019

PR#439 works for hip-clang. So drop this PR. We need PR#439 merged ASAP since it is blocking hip-clang CI. Thanks.

@yxsamliu yxsamliu closed this May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whchung whchung whchung requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yxsamliu @ROCm-Apps-Test @whchung