Skip to content

[LIBCLC][HIP] Add HIP AMD support for ilogb, log2, remainder #5272

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

Merged
merged 2 commits into from
Feb 2, 2022

Conversation

hdelan
Copy link
Contributor

@hdelan hdelan commented Jan 7, 2022

Fixing bug due to incorrect naming of remainder.cl as remainer.cl.

@hdelan hdelan requested a review from bader as a code owner January 7, 2022 15:32
@hdelan hdelan changed the title [LIBCLC][HIP] Wiring up support for ilogb, log2, remainder [LIBCLC][HIP] Add HIP AMD support for ilogb, log2, remainder Jan 7, 2022
bader
bader previously approved these changes Jan 10, 2022
bader
bader previously approved these changes Jan 11, 2022
@AerialMantis
Copy link
Contributor

@bader could we get another review on this one? I also see that one of the CI jobs failed but the failure doesn't seem related, would you be able to re-run the CI.

@bader
Copy link
Contributor

bader commented Feb 1, 2022

@bader could we get another review on this one? I also see that one of the CI jobs failed but the failure doesn't seem related, would you be able to re-run the CI.

I did this a few weeks ago and it failed after re-run. I just re-started CI jobs again, let's see if it passes on third attempt. If not, I would recommend pulling sycl branch to https://github.com/hdelan/llvm/tree/hugh/libclc_amd.

I missed the review because it's doesn't show up on my GitHub dashboard - https://github.com/pulls/review-requested. I would appreciate if you can request via GitHub UI.
image
Let me know if it's not available for you. I usually track PRs where my review is explicitly requested. It's hard to track all PRs.

@bader
Copy link
Contributor

bader commented Feb 2, 2022

The test still fails. @hdelan, could you update the branch, please?

@hdelan
Copy link
Contributor Author

hdelan commented Feb 2, 2022

Branch updated! Sorry for delay

@hdelan hdelan requested a review from bader February 2, 2022 10:59
@bader bader merged commit faaba28 into intel:sycl Feb 2, 2022
@AerialMantis
Copy link
Contributor

Thanks @bader no worries, yeah I can do that instead, though I don't see that option, maybe only the author of the PR can see that?

@bader
Copy link
Contributor

bader commented Feb 3, 2022

I don't see that option, maybe only the author of the PR can see that?

Do you see that option in closed PR or you don't see in in open PRs as well? Do you see it here: #5303?

@AerialMantis
Copy link
Contributor

Okay yeah I can see it on that one, I did check another open PR but I think it only shows if there's been changes since that person's reviews, thanks :)

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.

3 participants