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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
