Page MenuHomePhabricator

[OpenMP][AMDGCN] Apply fix for isnan, isinf and isfinite for amdgcn.
ClosedPublic

Authored by estewart08 on Jun 21 2021, 6:22 PM.

Details

Summary

This fixes issues with various return types(bool/int) and was already
in place for nvptx headers, adjusted to work for amdgcn. This does
not affect hip as the change is guarded with OPENMP_AMDGCN.
Similar to D85879.

Diff Detail

Event Timeline

estewart08 created this revision.Jun 21 2021, 6:22 PM
estewart08 requested review of this revision.Jun 21 2021, 6:22 PM
Herald added a project: Restricted Project. · View Herald Transcript
estewart08 added a project: Restricted Project.

Attempt to use clang-format.

estewart08 retitled this revision from [OpenMP] Apply fix for isnan, isinf and isinfinite for amdgcn. to [OpenMP][AMDGCN] Apply fix for isnan, isinf and isinfinite for amdgcn..Jun 21 2021, 7:09 PM
estewart08 edited the summary of this revision. (Show Details)
estewart08 retitled this revision from [OpenMP][AMDGCN] Apply fix for isnan, isinf and isinfinite for amdgcn. to [OpenMP][AMDGCN] Apply fix for isnan, isinf and isfinite for amdgcn..Jun 21 2021, 7:18 PM
jdoerfert accepted this revision.Jun 21 2021, 10:18 PM

LGTM, glad AMD GPUs are catching up on the math header support now.

This revision is now accepted and ready to land.Jun 21 2021, 10:18 PM

Looks very similar to the cuda workaround. Any chance we can #include to use the same code in both?

JonChesterfield accepted this revision.Jun 22 2021, 4:59 AM
yaxunl requested changes to this revision.Jun 22 2021, 7:17 AM

can we add a test to https://github.com/llvm/llvm-project/blob/main/clang/test/Headers/hip-header.hip just to make sure these functions are still available for HIP? Thanks.

This revision now requires changes to proceed.Jun 22 2021, 7:17 AM
ashi1 added a reviewer: tra.Jun 22 2021, 7:32 AM

Looks OK to me, please address other review comments. Thanks.

JonChesterfield added a comment.EditedJun 22 2021, 12:14 PM

Looks like the __OPENMP_AMDGCN__ guard is sound to me, but can't really argue against having more tests

Add test_isnan function to hip-header.hip.
yaxunl added inline comments.Jun 23 2021, 6:48 AM
clang/test/Headers/hip-header.hip
21 ↗(On Diff #353773)

where is this macro used and how does it affect HIP? Thanks.

estewart08 added inline comments.Jun 23 2021, 7:05 AM
clang/test/Headers/hip-header.hip
21 ↗(On Diff #353773)
yaxunl accepted this revision.Jun 23 2021, 7:07 AM

LGTM. Thanks.

This revision is now accepted and ready to land.Jun 23 2021, 7:07 AM