[PATCH] D36580: [OpenCL] Support variable memory scope in atomic builtins
Yaxun Liu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Aug 13 20:02:39 PDT 2017
yaxunl marked 6 inline comments as done.
yaxunl added inline comments.
================
Comment at: lib/CodeGen/CGAtomic.cpp:678
+ auto &Builder = CGF.Builder;
+ auto Scopes = getAllLanguageSyncScopes();
+ llvm::DenseMap<unsigned, llvm::BasicBlock *> BB;
----------------
t-tye wrote:
> yaxunl wrote:
> > t-tye wrote:
> > > Should only the scopes that apply to the language be returned otherwise will be generating code for invalid (possibly duplicate ABI) values?
> > getAllLanguageSyncScopes() only returns scope values for current language. I will rename it to getRuntimeSyncScopeValuesForCurrentLanguage() to avoid confusing.
> Curretly getAllLanguageSyncScopes does not take a LangOpt so not sure how it knows what the language is, and did not see it checking that the language is OpenCL internally. For non-OpenCL languages do they still have to support system scope?
For now I just assume all languages use OpenCL memory scope ABI. If a language has its own memory scope ABI it can be added later.
================
Comment at: lib/Frontend/InitPreprocessor.cpp:582
// The values should match clang SyncScope enum.
- assert(static_cast<unsigned>(SyncScope::OpenCLWorkGroup) == 1 &&
- static_cast<unsigned>(SyncScope::OpenCLDevice) == 2 &&
- static_cast<unsigned>(SyncScope::OpenCLAllSVMDevices) == 3 &&
- static_cast<unsigned>(SyncScope::OpenCLSubGroup) == 4);
+ assert(static_cast<unsigned>(OpenCLMemoryScope::WorkGroup) == 1 &&
+ static_cast<unsigned>(OpenCLMemoryScope::Device) == 2 &&
----------------
t-tye wrote:
> Could this be static_assert?
Will do.
https://reviews.llvm.org/D36580
More information about the cfe-commits
mailing list