This is an archive of the discontinued LLVM Phabricator instance.

[HIP] Add HIP scope atomic operations
ClosedPublic

Authored by gandhi21299 on Nov 15 2021, 11:15 AM.

Details

Summary

Add an AtomicScopeModel for HIP and support for OpenCL builtins
that are missing in HIP.

Diff Detail

Event Timeline

gandhi21299 created this revision.Nov 15 2021, 11:15 AM
gandhi21299 requested review of this revision.Nov 15 2021, 11:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2021, 11:15 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

clang-formatted code

reapplied clang-format

yaxunl added inline comments.Nov 16 2021, 6:29 AM
clang/lib/CodeGen/CGAtomic.cpp
1347–1348

debugging code should be removed

clang/lib/Frontend/InitPreprocessor.cpp
515–519

these should be moved outside of if (LangOpts.CUDAIsDevice) as they need to be visible to both device and host compilation. The downstream amd-stg-open branch already did this.

gandhi21299 marked an inline comment as done.
  • removed debug code
  • some macro definitions need to be defined for HIP-only compilation
gandhi21299 marked an inline comment as done.Nov 16 2021, 8:53 AM
yaxunl accepted this revision.Nov 18 2021, 12:31 PM

LGTM. Thanks.

This revision is now accepted and ready to land.Nov 18 2021, 12:31 PM

@yaxunl thanks for the review! My Github account is locked unfortunately so I will have to ask you to push this commit to the main branch. Thank you!

@yaxunl thanks for the review! My Github account is locked unfortunately so I will have to ask you to push this commit to the main branch. Thank you!

Sure.

This revision was landed with ongoing or failed builds.Nov 23 2021, 7:14 AM
This revision was automatically updated to reflect the committed changes.
rjmccall added inline comments.Nov 24 2021, 1:32 PM
clang/lib/Sema/SemaChecking.cpp
5587

I'm confused about what's happening here. You're editing comments to make more tendentious claims, and then adding FIXMEs to make charged questions about your own claims? Did you consider just not editing the comments?

gandhi21299 added inline comments.Nov 24 2021, 3:11 PM
clang/lib/Sema/SemaChecking.cpp
5587

Ahh I totally missed this, my apologies. I will revert this patch and get rid of this comment. Thanks for pointing it out.

gandhi21299 added inline comments.Nov 24 2021, 4:07 PM
clang/lib/Sema/SemaChecking.cpp
5587

Actually, this comes from a different patch: https://reviews.llvm.org/D114025#change-dkrDg1ZRTmhm

rjmccall added inline comments.Nov 24 2021, 6:49 PM
clang/lib/Sema/SemaChecking.cpp
5587

Ah, Phabricator is just being weird, then; sorry for the noise.