[OpenCL] Support variable memory scope in atomic builtins

[rjmccall]

include/clang/Basic/SyncScope.h

47 ↗

(On Diff #110575)

You could just return an ArrayRef to a static const array.

lib/CodeGen/CGAtomic.cpp

687 ↗

(On Diff #110575)

Does Sema not coerce the argument to int? It really should, and then you can just rely on that here. (You should use CGF.IntTy if you do this, though, in case you're on a target with a non-32-bit int.)

[t-tye]

include/clang/Basic/SyncScope.h

46 ↗

(On Diff #110575)

Should there be an assert/static_assert in case SyncScope enum grows due to other languages?

59 ↗

(On Diff #110575)

Should there be a default/assert/static_assert to allow SyncScope enum to grow due to other languages?

lib/CodeGen/CGAtomic.cpp

696 ↗

(On Diff #110575)

Is it documented in the SyncScope enum that the enumerator values are in fact the values used for source language runtime values? Seems if other languages want to use scopes they may may have a different ordering. That would imply that there would be a function to map a SyncScope value to the value used by the source language. For OpenCL the mapping is identity.

The memory ordering has the isValidAtomicOrderingCABI() that does a similar thing.

[rjmccall]

include/clang/Basic/SyncScope.h

46 ↗

(On Diff #110575)

It makes sense to add a 'last' enumerator to SyncScope and do a static_assert here that last == OpenCLSubGroup.

59 ↗

(On Diff #110575)

There will already be a warning from -Wswitch about this, which should be sufficient.

But we do often add llvm_unreachable after switches like this.

lib/CodeGen/CGAtomic.cpp

696 ↗

(On Diff #110575)

The values in the SyncScope enum are the source language values. We already have a step to translate them into LLVM values when we generate a native LLVM construct. To the extent that we call into a runtime instead, none of that code has been written to be runtime-agnostic at all, and I've been assuming that we're basically okay with that, at least for now.

[t-tye]

LGTM other than suggested documentation and static_assert/unreachable comments.

lib/CodeGen/CGAtomic.cpp

696 ↗

(On Diff #110575)

That sounds reasonable to me. When/if another language comes along the task of mapping the multiple ABIs can be done then. Still wanted to make sure it is clear that the enum values in the SyncScope enum are documented as BEING the OpenCL runtime ABI values and not some arbitrary list as in other enums such as the address space enum.

[rjmccall]

lib/CodeGen/CGAtomic.cpp

696 ↗

(On Diff #110575)

They are documented as being the values expected by the builtins, so they already can't be arbitrarily changed.

Now, the values expected by the builtin were chosen to match this specific runtime, but we had to choose something, and matching a runtime isn't the worst way of doing that. But now that I think about it, those values seem to be rather sub-optimal because they don't start at 0, which means that things like this switch will be less efficient than they could be. And I think the AST and IRGen would benefit from needing to renumber values; it will make the code clearer, and it'll help protect against people adding values to this enum just because those values are used by the runtime, i.e. without fully realizing that they're expanding a user-facing feature. So let's go ahead and renumber the values in SyncScope to start at 0 and then re-map them in IRGen.

[yaxunl]

include/clang/Basic/SyncScope.h

46 ↗

(On Diff #110575)

Thanks. Will do.

47 ↗

(On Diff #110575)

Thanks. Will do.

59 ↗

(On Diff #110575)

I tried adding

default:
    llvm_unreachable("Invalid sync scope");

However, it results in error:

error: default label in switch which covers all enumeration values [-Werror,-Wcovered-switch-default]

I guess I'd better leave it as it is since we already have -Wswitch.

lib/CodeGen/CGAtomic.cpp

687 ↗

(On Diff #110575)

It is coerced to Context.IntTy. I will add some tests for non-integral order/scope argument.

696 ↗

(On Diff #110575)

Will do. Thanks.

[yaxunl]

Revised by John's and Tony's comments.

[t-tye]

include/clang/Basic/SyncScope.h

56 ↗

(On Diff #110863)

AtomicScopeOpenCLABI to mirror AtomicOrderingCABI?

94 ↗

(On Diff #110863)

Should this take a LangOpt since different languages may use different value ABIs?

lib/CodeGen/CGAtomic.cpp

678 ↗

(On Diff #110863)

Should only the scopes that apply to the language be returned otherwise will be generating code for invalid (possibly duplicate ABI) values?

lib/Frontend/InitPreprocessor.cpp

582 ↗

(On Diff #110863)

Could this be static_assert?

[yaxunl]

include/clang/Basic/SyncScope.h

56 ↗

(On Diff #110863)

Will do.

94 ↗

(On Diff #110863)

Although currently this function does not use LangOpt, but I agree it may be a good idea to make it future proof. Will add it and update comment.

lib/CodeGen/CGAtomic.cpp

678 ↗

(On Diff #110863)

getAllLanguageSyncScopes() only returns scope values for current language. I will rename it to getRuntimeSyncScopeValuesForCurrentLanguage() to avoid confusing.

[t-tye]

lib/CodeGen/CGAtomic.cpp

678 ↗

(On Diff #110863)

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?

[yaxunl]

lib/CodeGen/CGAtomic.cpp

678 ↗

(On Diff #110863)

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.

lib/Frontend/InitPreprocessor.cpp

582 ↗

(On Diff #110863)

Will do.

[yaxunl]

Refactor to introduce classes AtomicScopeABI and AtomicScopeOpenCLABI for easy extension to other languages.

[t-tye]

LGTM

[rjmccall]

include/clang/Basic/SyncScope.h

89 ↗

(On Diff #110904)

The previous design of this header seems more appropriate to me. Clang defines a builtin with standardized argument values for all targets; we're not trying to allow targets to customize the arguments taken by this builtin, at least not yet. The sync-scope argument values are the values from this enum. Because they are potentially exposed in source programs, they cannot be lightly changed and re-ordered; that is, they are ABI. There is a second level of ABI, which is the set of values demanded by target runtime functions. It is IRGen's responsibility to turn the SyncScope values into those values when generating a call to the runtime; that mapping does not need to be exposed here in the AST.

92 ↗

(On Diff #110904)

Please correct me if I'm wrong, but I believe you don't get to define the "ABI for OpenCL" any more than I get to define the "ABI for C". You're defining the ABI for your specific OpenCL implementation, but Clang might reasonably support other implementations with their own ABIs. So this name is inappropriate.

Now, if I understand how SPIR works, SPIR does require some sort of stable lowering of these atomic operations in order to ensure that the resulting LLVM IR can be ingested into an arbitrary implementation. (Ot at least that was true prior to SPIR V?) Therefore, SPIR needs to specify a lowering for these atomic operations, and that probably includes ABI values for specific sync-scopes. But the appropriate name for that would be the "SPIR ABI", not the "OpenCL ABI". And, of course, you would need to be sure that you're implementing the lowering that SPIR actually wants.

59 ↗

(On Diff #110575)

Yeah, the idiom is to put the llvm_unreachable after the switch, not in a default. Check out the uses in Type.cpp for examples: some of them are in specific cases, saying those cases are illlegal due to some precondition, but most of them are just after the switch saying that the switch is covered. (The semantics of switch in C are that switch values with no corresponding case just skip the whole switch unless there's a default.)

[yaxunl]

include/clang/Basic/SyncScope.h

89 ↗

(On Diff #110904)

This function is not intended to create ABI's for different targets, but to create ABI's for different languages. Currently only OpenCL supports atomic scope, but in the future there may be other languages which support atomic scope with a different ABI.

92 ↗

(On Diff #110904)

The ABI is not intended for a specific OpenCL implementation, but for extending to other languages. For example, we have a downstream C++ based language called HCC, which may support atomic scope with an ABI different than OpenCL atomic scope ABI.

[rjmccall]

include/clang/Basic/SyncScope.h

92 ↗

(On Diff #110904)

By "ABI" you mean that it might present a different model of synchronization scopes to the source language? Okay, that's great, and it explains a lot of things that were very murky about some of our previous conversations.

In that case, I think the appropriate design for these builtins is:

• They don't need to be in the __builtin_opencl namespace.
• They *do* need to be enabled only in language modes with well-defined sync-scope models, which for now just means OpenCL.
• The language mode's sync-scope model should determine everything about the scope argument, including its type. Sema-level validation requires first determining the language's sync-scope model.
• There is no meaningful way to "default" the scope argument on the non-scoped builtins the way that we are now. Instead, the scope argument should only exist for the scoped builtins.

Alternatively, if you potentially want the opencl-model builtins to be usable from non-opencl languages, the right design is for HCC to use its own set of builtins with their own type-checking rules.

In either case, I don't think it's a good idea to call this "ABI", which is associated too strongly with target-lowering concepts. You're really talking about a source-language concept.

[yaxunl]

include/clang/Basic/SyncScope.h

92 ↗

(On Diff #110904)

We could introduce __builtin_hcc_atomic_* in downstream HCC.

We need a generic approach to represent atomic scope values for different languages and associated semantic/codegen needs, and AtomicScopeABI seems generic enough to serve the purpose. It has interface for semantic checking (isValid), and the interface to map to LLVM sync scope for both constant and variable arguments. If the name is not proper, we could rename it to just AtomicScope.

I can remove the default atomic scope for atomic expressions without scope originally. For them I just emit LLVM atomic instructions with default LLVM synch scope.

[rjmccall]

include/clang/Basic/SyncScope.h

92 ↗

(On Diff #110904)

I agree that there's value in having a generic approach even if you're going to have different builtins for different scope models. However, if you do use different builtins for different scope models, you need to make sure that you pick the appropriate model for the builtin being used, not according to the current language mode. That is, there should be some way to map a builtin ID to an AtomicScopeModelKind (values: None, OpenCL, (future) HCC), and then you can use that to create an AtomicScopeModel, which can have all these methods on it.

I would encourage you to go ahead and give AtomicScopeModel an implementation file and move more of these method definitions into that.

105 ↗

(On Diff #110904)

I would call this OpenCLAtomicScopeModel.

128 ↗

(On Diff #110904)

Make this const, please.

[yaxunl]

include/clang/Basic/SyncScope.h

92 ↗

(On Diff #110904)

Thanks. Will do.

105 ↗

(On Diff #110904)

Will do.

128 ↗

(On Diff #110904)

will do.

[yaxunl]

Revised by John's comments.

[rjmccall]

Thanks, that looks great.

[t-tye]

cfe/trunk/lib/Frontend/InitPreprocessor.cpp

581

// The values should match clang AtomicScopeOpenCLModel::ID enum.

[yaxunl]

cfe/trunk/lib/Frontend/InitPreprocessor.cpp

581

Fixed. Thanks.

cfe/trunk/include/clang/AST/Expr.h

 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclAccessPair.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SyncScope.h"
 #include "clang/Basic/TypeTraits.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/Compiler.h"
 
public:
   friend class ASTStmtReader;
 };
 
 /// AtomicExpr - Variadic atomic builtins: __atomic_exchange, __atomic_fetch_*,
 /// __atomic_load, __atomic_store, and __atomic_compare_exchange_*, for the
 /// similarly-named C++11 instructions, and __c11 variants for <stdatomic.h>,
 /// and corresponding __opencl_atomic_* for OpenCL 2.0.
 /// All of these instructions take one primary pointer, at least one memory
-/// order, and one synchronization scope. The C++11 and __c11 atomic AtomicExpr
-/// always take the default synchronization scope.
+/// order. The instructions for which getScopeModel returns non-null value
+/// take one synch scope.
 class AtomicExpr : public Expr {
 public:
   enum AtomicOp {
 #define BUILTIN(ID, TYPE, ATTRS)
 #define ATOMIC_BUILTIN(ID, TYPE, ATTRS) AO ## ID,
 #include "clang/Basic/Builtins.def"
     // Avoid trailing comma
     BI_First = 0
   };
 
 private:
-  enum { PTR, ORDER, SCOPE, VAL1, ORDER_FAIL, VAL2, WEAK, END_EXPR };
-  Stmt* SubExprs[END_EXPR];
+  /// \brief Location of sub-expressions.
+  /// The location of Scope sub-expression is NumSubExprs - 1, which is
+  /// not fixed, therefore is not defined in enum.
+  enum { PTR, ORDER, VAL1, ORDER_FAIL, VAL2, WEAK, END_EXPR };
+  Stmt *SubExprs[END_EXPR + 1];
   unsigned NumSubExprs;
   SourceLocation BuiltinLoc, RParenLoc;
   AtomicOp Op;
 
   friend class ASTStmtReader;
-
 public:
   AtomicExpr(SourceLocation BLoc, ArrayRef<Expr*> args, QualType t,
              AtomicOp op, SourceLocation RP);
 
   /// \brief Determine the number of arguments the specified atomic builtin
   /// should have.
   static unsigned getNumSubExprs(AtomicOp Op);
 
   /// \brief Build an empty AtomicExpr.
   explicit AtomicExpr(EmptyShell Empty) : Expr(AtomicExprClass, Empty) { }
 
   Expr *getPtr() const {
     return cast<Expr>(SubExprs[PTR]);
   }
   Expr *getOrder() const {
     return cast<Expr>(SubExprs[ORDER]);
   }
   Expr *getScope() const {
-    return cast<Expr>(SubExprs[SCOPE]);
+    assert(getScopeModel() && "No scope");
+    return cast<Expr>(SubExprs[NumSubExprs - 1]);
   }
   Expr *getVal1() const {
     if (Op == AO__c11_atomic_init || Op == AO__opencl_atomic_init)
       return cast<Expr>(SubExprs[ORDER]);
     assert(NumSubExprs > VAL1);
     return cast<Expr>(SubExprs[VAL1]);
   }
   Expr *getOrderFail() const {
public:
 
   // Iterators
   child_range children() {
     return child_range(SubExprs, SubExprs+NumSubExprs);
   }
   const_child_range children() const {
     return const_child_range(SubExprs, SubExprs + NumSubExprs);
   }
+
+  /// \brief Get atomic scope model for the atomic op code.
+  /// \return empty atomic scope model if the atomic op code does not have
+  ///   scope operand.
+  static std::unique_ptr<AtomicScopeModel> getScopeModel(AtomicOp Op) {
+    auto Kind =
+        (Op >= AO__opencl_atomic_load && Op <= AO__opencl_atomic_fetch_max)
+            ? AtomicScopeModelKind::OpenCL
+            : AtomicScopeModelKind::None;
+    return AtomicScopeModel::create(Kind);
+  }
+
+  /// \brief Get atomic scope model.
+  /// \return empty atomic scope model if this atomic expression does not have
+  ///   scope operand.
+  std::unique_ptr<AtomicScopeModel> getScopeModel() const {
+    return getScopeModel(getOp());
+  }
 };
 
 /// TypoExpr - Internal placeholder for expressions where typo correction
 /// still needs to be performed and/or an error diagnostic emitted.
 class TypoExpr : public Expr {
 public:
   TypoExpr(QualType T)
       : Expr(TypoExprClass, T, VK_LValue, OK_Ordinary,

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

 def err_atomic_op_bitwise_needs_atomic_int : Error<
   "address argument to bitwise atomic operation must be a pointer to "
   "%select{|atomic }0integer (%1 invalid)">;
 def warn_atomic_op_has_invalid_memory_order : Warning<
   "memory order argument to atomic operation is invalid">,
   InGroup<DiagGroup<"atomic-memory-ordering">>;
 def err_atomic_op_has_invalid_synch_scope : Error<
   "synchronization scope argument to atomic operation is invalid">;
-def err_atomic_op_has_non_constant_synch_scope : Error<
-  "non-constant synchronization scope argument to atomic operation is not supported">;
 
 def err_overflow_builtin_must_be_int : Error<
   "operand argument to overflow builtin must be an integer (%0 invalid)">;
 def err_overflow_builtin_must_be_ptr_int : Error<
   "result argument to overflow builtin must be a pointer "
   "to a non-const integer (%0 invalid)">;
 
 def err_atomic_load_store_uses_lib : Error<

cfe/trunk/include/clang/Basic/SyncScope.h

 /// \file
 /// \brief Provides definitions for the atomic synchronization scopes.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_SYNCSCOPE_H
 #define LLVM_CLANG_BASIC_SYNCSCOPE_H
 
+#include "clang/Basic/LangOptions.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+#include <memory>
+
 namespace clang {
 
-/// \brief Defines the synch scope values used by the atomic builtins and
-/// expressions.
+/// \brief Defines synch scope values used internally by clang.
+///
+/// The enum values start from 0 and are contiguous. They are mainly used for
+/// enumerating all supported synch scope values and mapping them to LLVM
+/// synch scopes. Their numerical values may be different from the corresponding
+/// synch scope enums used in source languages.
+///
+/// In atomic builtin and expressions, language-specific synch scope enums are
+/// used. Currently only OpenCL memory scope enums are supported and assumed
+/// to be used by all languages. However, in the future, other languages may
+/// define their own set of synch scope enums. The language-specific synch scope
+/// values are represented by class AtomicScopeModel and its derived classes.
+///
+/// To add a new enum value:
+///   Add the enum value to enum class SyncScope.
+///   Update enum value Last if necessary.
+///   Update getAsString.
 ///
-/// The enum values should match the pre-defined macros
+enum class SyncScope {
+  OpenCLWorkGroup,
+  OpenCLDevice,
+  OpenCLAllSVMDevices,
+  OpenCLSubGroup,
+  Last = OpenCLSubGroup
+};
+
+inline llvm::StringRef getAsString(SyncScope S) {
+  switch (S) {
+  case SyncScope::OpenCLWorkGroup:
+    return "opencl_workgroup";
+  case SyncScope::OpenCLDevice:
+    return "opencl_device";
+  case SyncScope::OpenCLAllSVMDevices:
+    return "opencl_allsvmdevices";
+  case SyncScope::OpenCLSubGroup:
+    return "opencl_subgroup";
+  }
+  llvm_unreachable("Invalid synch scope");
+}
+
+/// \brief Defines the kind of atomic scope models.
+enum class AtomicScopeModelKind { None, OpenCL };
+
+/// \brief Defines the interface for synch scope model.
+class AtomicScopeModel {
+public:
+  virtual ~AtomicScopeModel() {}
+  /// \brief Maps language specific synch scope values to internal
+  /// SyncScope enum.
+  virtual SyncScope map(unsigned S) const = 0;
+
+  /// \brief Check if the compile-time constant synch scope value
+  /// is valid.
+  virtual bool isValid(unsigned S) const = 0;
+
+  /// \brief Get all possible synch scope values that might be
+  /// encountered at runtime for the current language.
+  virtual ArrayRef<unsigned> getRuntimeValues() const = 0;
+
+  /// \brief If atomic builtin function is called with invalid
+  /// synch scope value at runtime, it will fall back to a valid
+  /// synch scope value returned by this function.
+  virtual unsigned getFallBackValue() const = 0;
+
+  /// \brief Create an atomic scope model by AtomicScopeModelKind.
+  /// \return an empty std::unique_ptr for AtomicScopeModelKind::None.
+  static std::unique_ptr<AtomicScopeModel> create(AtomicScopeModelKind K);
+};
+
+/// \brief Defines the synch scope model for OpenCL.
+class AtomicScopeOpenCLModel : public AtomicScopeModel {
+public:
+  /// The enum values match the pre-defined macros
   /// __OPENCL_MEMORY_SCOPE_*, which are used to define memory_scope_*
   /// enums in opencl-c.h.
-enum class SyncScope {
-  OpenCLWorkGroup = 1,
-  OpenCLDevice = 2,
-  OpenCLAllSVMDevices = 3,
-  OpenCLSubGroup = 4,
+  enum ID {
+    WorkGroup = 1,
+    Device = 2,
+    AllSVMDevices = 3,
+    SubGroup = 4,
+    Last = SubGroup
   };
 
-inline bool isValidSyncScopeValue(unsigned Scope) {
-  return Scope >= static_cast<unsigned>(SyncScope::OpenCLWorkGroup) &&
-         Scope <= static_cast<unsigned>(SyncScope::OpenCLSubGroup);
+  AtomicScopeOpenCLModel() {}
+
+  SyncScope map(unsigned S) const override {
+    switch (static_cast<ID>(S)) {
+    case WorkGroup:
+      return SyncScope::OpenCLWorkGroup;
+    case Device:
+      return SyncScope::OpenCLDevice;
+    case AllSVMDevices:
+      return SyncScope::OpenCLAllSVMDevices;
+    case SubGroup:
+      return SyncScope::OpenCLSubGroup;
+    }
+    llvm_unreachable("Invalid language synch scope value");
+  }
+
+  bool isValid(unsigned S) const override {
+    return S >= static_cast<unsigned>(WorkGroup) &&
+           S <= static_cast<unsigned>(Last);
+  }
+
+  ArrayRef<unsigned> getRuntimeValues() const override {
+    static_assert(Last == SubGroup, "Does not include all synch scopes");
+    static const unsigned Scopes[] = {
+        static_cast<unsigned>(WorkGroup), static_cast<unsigned>(Device),
+        static_cast<unsigned>(AllSVMDevices), static_cast<unsigned>(SubGroup)};
+    return llvm::makeArrayRef(Scopes);
+  }
+
+  unsigned getFallBackValue() const override {
+    return static_cast<unsigned>(AllSVMDevices);
+  }
+};
+
+inline std::unique_ptr<AtomicScopeModel>
+AtomicScopeModel::create(AtomicScopeModelKind K) {
+  switch (K) {
+  case AtomicScopeModelKind::None:
+    return std::unique_ptr<AtomicScopeModel>{};
+  case AtomicScopeModelKind::OpenCL:
+    return llvm::make_unique<AtomicScopeOpenCLModel>();
+  }
+  llvm_unreachable("Invalid atomic scope model kind");
 }
 }
 
 #endif

cfe/trunk/lib/AST/Expr.cpp

for (unsigned i = 0; i != args.size(); i++) {
     SubExprs[i] = args[i];
   }
 }
 
 unsigned AtomicExpr::getNumSubExprs(AtomicOp Op) {
   switch (Op) {
   case AO__c11_atomic_init:
   case AO__opencl_atomic_init:
-    return 2;
   case AO__c11_atomic_load:
-  case AO__opencl_atomic_load:
   case AO__atomic_load_n:
-    return 3;
+    return 2;
 
+  case AO__opencl_atomic_load:
   case AO__c11_atomic_store:
   case AO__c11_atomic_exchange:
-  case AO__opencl_atomic_store:
-  case AO__opencl_atomic_exchange:
   case AO__atomic_load:
   case AO__atomic_store:
   case AO__atomic_store_n:
   case AO__atomic_exchange_n:
   case AO__c11_atomic_fetch_add:
   case AO__c11_atomic_fetch_sub:
   case AO__c11_atomic_fetch_and:
   case AO__c11_atomic_fetch_or:
   case AO__c11_atomic_fetch_xor:
-  case AO__opencl_atomic_fetch_add:
-  case AO__opencl_atomic_fetch_sub:
-  case AO__opencl_atomic_fetch_and:
-  case AO__opencl_atomic_fetch_or:
-  case AO__opencl_atomic_fetch_xor:
-  case AO__opencl_atomic_fetch_min:
-  case AO__opencl_atomic_fetch_max:
   case AO__atomic_fetch_add:
   case AO__atomic_fetch_sub:
   case AO__atomic_fetch_and:
   case AO__atomic_fetch_or:
   case AO__atomic_fetch_xor:
   case AO__atomic_fetch_nand:
   case AO__atomic_add_fetch:
   case AO__atomic_sub_fetch:
   case AO__atomic_and_fetch:
   case AO__atomic_or_fetch:
   case AO__atomic_xor_fetch:
   case AO__atomic_nand_fetch:
-    return 4;
+    return 3;
 
+  case AO__opencl_atomic_store:
+  case AO__opencl_atomic_exchange:
+  case AO__opencl_atomic_fetch_add:
+  case AO__opencl_atomic_fetch_sub:
+  case AO__opencl_atomic_fetch_and:
+  case AO__opencl_atomic_fetch_or:
+  case AO__opencl_atomic_fetch_xor:
+  case AO__opencl_atomic_fetch_min:
+  case AO__opencl_atomic_fetch_max:
   case AO__atomic_exchange:
-    return 5;
+    return 4;
 
   case AO__c11_atomic_compare_exchange_strong:
   case AO__c11_atomic_compare_exchange_weak:
+    return 5;
+
   case AO__opencl_atomic_compare_exchange_strong:
   case AO__opencl_atomic_compare_exchange_weak:
-    return 6;
-
   case AO__atomic_compare_exchange:
   case AO__atomic_compare_exchange_n:
-    return 7;
+    return 6;
   }
   llvm_unreachable("unknown atomic op");
 }
 
 QualType AtomicExpr::getValueType() const {
   auto T = getPtr()->getType()->castAs<PointerType>()->getPointeeType();
   if (auto AT = T->getAs<AtomicType>())
     return AT->getValueType();

cfe/trunk/lib/CodeGen/CGAtomic.cpp

 
 #include "CGCall.h"
 #include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Operator.h"
 
 using namespace clang;
 using namespace CodeGen;
 
 namespace {

 static Address
 EmitValToTemp(CodeGenFunction &CGF, Expr *E) {
   Address DeclPtr = CGF.CreateMemTemp(E->getType(), ".atomictmp");
   CGF.EmitAnyExprToMem(E, DeclPtr, E->getType().getQualifiers(),
                        /*Init*/ true);
   return DeclPtr;
 }
 
+static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *Expr, Address Dest,
+                         Address Ptr, Address Val1, Address Val2,
+                         llvm::Value *IsWeak, llvm::Value *FailureOrder,
+                         uint64_t Size, llvm::AtomicOrdering Order,
+                         llvm::Value *Scope) {
+  auto ScopeModel = Expr->getScopeModel();
+
+  // LLVM atomic instructions always have synch scope. If clang atomic
+  // expression has no scope operand, use default LLVM synch scope.
+  if (!ScopeModel) {
+    EmitAtomicOp(CGF, Expr, Dest, Ptr, Val1, Val2, IsWeak, FailureOrder, Size,
+                 Order, CGF.CGM.getLLVMContext().getOrInsertSyncScopeID(""));
+    return;
+  }
+
+  // Handle constant scope.
+  if (auto SC = dyn_cast<llvm::ConstantInt>(Scope)) {
+    auto SCID = CGF.getTargetHooks().getLLVMSyncScopeID(
+        ScopeModel->map(SC->getZExtValue()), CGF.CGM.getLLVMContext());
+    EmitAtomicOp(CGF, Expr, Dest, Ptr, Val1, Val2, IsWeak, FailureOrder, Size,
+                 Order, SCID);
+    return;
+  }
+
+  // Handle non-constant scope.
+  auto &Builder = CGF.Builder;
+  auto Scopes = ScopeModel->getRuntimeValues();
+  llvm::DenseMap<unsigned, llvm::BasicBlock *> BB;
+  for (auto S : Scopes)
+    BB[S] = CGF.createBasicBlock(getAsString(ScopeModel->map(S)), CGF.CurFn);
+
+  llvm::BasicBlock *ContBB =
+      CGF.createBasicBlock("atomic.scope.continue", CGF.CurFn);
+
+  auto *SC = Builder.CreateIntCast(Scope, Builder.getInt32Ty(), false);
+  // If unsupported synch scope is encountered at run time, assume a fallback
+  // synch scope value.
+  auto FallBack = ScopeModel->getFallBackValue();
+  llvm::SwitchInst *SI = Builder.CreateSwitch(SC, BB[FallBack]);
+  for (auto S : Scopes) {
+    auto *B = BB[S];
+    if (S != FallBack)
+      SI->addCase(Builder.getInt32(S), B);
+
+    Builder.SetInsertPoint(B);
+    EmitAtomicOp(CGF, Expr, Dest, Ptr, Val1, Val2, IsWeak, FailureOrder, Size,
+                 Order,
+                 CGF.getTargetHooks().getLLVMSyncScopeID(ScopeModel->map(S),
+                                                         CGF.getLLVMContext()));
+    Builder.CreateBr(ContBB);
+  }
+
+  Builder.SetInsertPoint(ContBB);
+}
+
 static void
 AddDirectArgument(CodeGenFunction &CGF, CallArgList &Args,
                   bool UseOptimizedLibcall, llvm::Value *Val, QualType ValTy,
                   SourceLocation Loc, CharUnits SizeInChars) {
   if (UseOptimizedLibcall) {
     // Load value and pass it to the function directly.
     CharUnits Align = CGF.getContext().getTypeAlignInChars(ValTy);
     int64_t SizeInBits = CGF.getContext().toBits(SizeInChars);
RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   if (E->getOp() == AtomicExpr::AO__c11_atomic_init ||
       E->getOp() == AtomicExpr::AO__opencl_atomic_init) {
     LValue lvalue = MakeAddrLValue(Ptr, AtomicTy);
     EmitAtomicInit(E->getVal1(), lvalue);
     return RValue::get(nullptr);
   }
 
   llvm::Value *Order = EmitScalarExpr(E->getOrder());
-  llvm::Value *Scope = EmitScalarExpr(E->getScope());
+  llvm::Value *Scope =
+      E->getScopeModel() ? EmitScalarExpr(E->getScope()) : nullptr;
 
   switch (E->getOp()) {
   case AtomicExpr::AO__c11_atomic_init:
   case AtomicExpr::AO__opencl_atomic_init:
     llvm_unreachable("Already handled above with EmitAtomicInit!");
 
   case AtomicExpr::AO__c11_atomic_load:
   case AtomicExpr::AO__opencl_atomic_load:
bool IsStore = E->getOp() == AtomicExpr::AO__c11_atomic_store ||
                  E->getOp() == AtomicExpr::AO__opencl_atomic_store ||
                  E->getOp() == AtomicExpr::AO__atomic_store ||
                  E->getOp() == AtomicExpr::AO__atomic_store_n;
   bool IsLoad = E->getOp() == AtomicExpr::AO__c11_atomic_load ||
                 E->getOp() == AtomicExpr::AO__opencl_atomic_load ||
                 E->getOp() == AtomicExpr::AO__atomic_load ||
                 E->getOp() == AtomicExpr::AO__atomic_load_n;
 
-  assert(isa<llvm::ConstantInt>(Scope) &&
-      "Non-constant synchronization scope not supported");
-  auto SCID = getTargetHooks().getLLVMSyncScopeID(
-      static_cast<SyncScope>(cast<llvm::ConstantInt>(Scope)->getZExtValue()),
-      getLLVMContext());
-
   if (isa<llvm::ConstantInt>(Order)) {
     auto ord = cast<llvm::ConstantInt>(Order)->getZExtValue();
     // We should not ever get to a case where the ordering isn't a valid C ABI
     // value, but it's hard to enforce that in general.
     if (llvm::isValidAtomicOrderingCABI(ord))
       switch ((llvm::AtomicOrderingCABI)ord) {
       case llvm::AtomicOrderingCABI::relaxed:
         EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail, Size,
-                     llvm::AtomicOrdering::Monotonic, SCID);
+                     llvm::AtomicOrdering::Monotonic, Scope);
         break;
       case llvm::AtomicOrderingCABI::consume:
       case llvm::AtomicOrderingCABI::acquire:
         if (IsStore)
           break; // Avoid crashing on code with undefined behavior
         EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail, Size,
-                     llvm::AtomicOrdering::Acquire, SCID);
+                     llvm::AtomicOrdering::Acquire, Scope);
         break;
       case llvm::AtomicOrderingCABI::release:
         if (IsLoad)
           break; // Avoid crashing on code with undefined behavior
         EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail, Size,
-                     llvm::AtomicOrdering::Release, SCID);
+                     llvm::AtomicOrdering::Release, Scope);
         break;
       case llvm::AtomicOrderingCABI::acq_rel:
         if (IsLoad || IsStore)
           break; // Avoid crashing on code with undefined behavior
         EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail, Size,
-                     llvm::AtomicOrdering::AcquireRelease, SCID);
+                     llvm::AtomicOrdering::AcquireRelease, Scope);
         break;
       case llvm::AtomicOrderingCABI::seq_cst:
         EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail, Size,
-                     llvm::AtomicOrdering::SequentiallyConsistent, SCID);
+                     llvm::AtomicOrdering::SequentiallyConsistent, Scope);
         break;
       }
     if (RValTy->isVoidType())
       return RValue::get(nullptr);
 
     return convertTempToRValue(
         Builder.CreateBitCast(Dest, ConvertTypeForMem(RValTy)->getPointerTo()),
         RValTy, E->getExprLoc());
RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   // doesn't matter unless someone is crazy enough to use something that
   // doesn't fold to a constant for the ordering.
   Order = Builder.CreateIntCast(Order, Builder.getInt32Ty(), false);
   llvm::SwitchInst *SI = Builder.CreateSwitch(Order, MonotonicBB);
 
   // Emit all the different atomics
   Builder.SetInsertPoint(MonotonicBB);
   EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail, Size,
-               llvm::AtomicOrdering::Monotonic, SCID);
+               llvm::AtomicOrdering::Monotonic, Scope);
   Builder.CreateBr(ContBB);
   if (!IsStore) {
     Builder.SetInsertPoint(AcquireBB);
     EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail, Size,
-                 llvm::AtomicOrdering::Acquire, SCID);
+                 llvm::AtomicOrdering::Acquire, Scope);
     Builder.CreateBr(ContBB);
     SI->addCase(Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
                 AcquireBB);
     SI->addCase(Builder.getInt32((int)llvm::AtomicOrderingCABI::acquire),
                 AcquireBB);
   }
   if (!IsLoad) {
     Builder.SetInsertPoint(ReleaseBB);
     EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail, Size,
-                 llvm::AtomicOrdering::Release, SCID);
+                 llvm::AtomicOrdering::Release, Scope);
     Builder.CreateBr(ContBB);
     SI->addCase(Builder.getInt32((int)llvm::AtomicOrderingCABI::release),
                 ReleaseBB);
   }
   if (!IsLoad && !IsStore) {
     Builder.SetInsertPoint(AcqRelBB);
     EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail, Size,
-                 llvm::AtomicOrdering::AcquireRelease, SCID);
+                 llvm::AtomicOrdering::AcquireRelease, Scope);
     Builder.CreateBr(ContBB);
     SI->addCase(Builder.getInt32((int)llvm::AtomicOrderingCABI::acq_rel),
                 AcqRelBB);
   }
   Builder.SetInsertPoint(SeqCstBB);
   EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail, Size,
-               llvm::AtomicOrdering::SequentiallyConsistent, SCID);
+               llvm::AtomicOrdering::SequentiallyConsistent, Scope);
   Builder.CreateBr(ContBB);
   SI->addCase(Builder.getInt32((int)llvm::AtomicOrderingCABI::seq_cst),
               SeqCstBB);
 
   // Cleanup and return
   Builder.SetInsertPoint(ContBB);
   if (RValTy->isVoidType())
     return RValue::get(nullptr);

cfe/trunk/lib/Frontend/InitPreprocessor.cpp

#undef TOSTR2
   Builder.defineMacro("__ATOMIC_RELAXED", "0");
   Builder.defineMacro("__ATOMIC_CONSUME", "1");
   Builder.defineMacro("__ATOMIC_ACQUIRE", "2");
   Builder.defineMacro("__ATOMIC_RELEASE", "3");
   Builder.defineMacro("__ATOMIC_ACQ_REL", "4");
   Builder.defineMacro("__ATOMIC_SEQ_CST", "5");
 
   // Define macros for the OpenCL memory scope.
   // 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);
+  static_assert(
+      static_cast<unsigned>(AtomicScopeOpenCLModel::WorkGroup) == 1 &&
+          static_cast<unsigned>(AtomicScopeOpenCLModel::Device) == 2 &&
+          static_cast<unsigned>(AtomicScopeOpenCLModel::AllSVMDevices) == 3 &&
+          static_cast<unsigned>(AtomicScopeOpenCLModel::SubGroup) == 4,
+      "Invalid OpenCL memory scope enum definition");
   Builder.defineMacro("__OPENCL_MEMORY_SCOPE_WORK_ITEM", "0");
   Builder.defineMacro("__OPENCL_MEMORY_SCOPE_WORK_GROUP", "1");
   Builder.defineMacro("__OPENCL_MEMORY_SCOPE_DEVICE", "2");
   Builder.defineMacro("__OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES", "3");
   Builder.defineMacro("__OPENCL_MEMORY_SCOPE_SUB_GROUP", "4");
 
   // Support for #pragma redefine_extname (Sun compatibility)
   Builder.defineMacro("__PRAGMA_REDEFINE_EXTNAME", "1");

cfe/trunk/lib/Sema/SemaChecking.cpp

InitializedEntity Entity =
         InitializedEntity::InitializeParameter(Context, Ty, false);
     ExprResult Arg = TheCall->getArg(i);
     Arg = PerformCopyInitialization(Entity, SourceLocation(), Arg);
     if (Arg.isInvalid())
       return true;
     TheCall->setArg(i, Arg.get());
   }
 
-  Expr *Scope;
-  if (Form != Init) {
-    if (IsOpenCL) {
-      Scope = TheCall->getArg(TheCall->getNumArgs() - 1);
-      llvm::APSInt Result(32);
-      if (!Scope->isIntegerConstantExpr(Result, Context))
-        Diag(Scope->getLocStart(),
-             diag::err_atomic_op_has_non_constant_synch_scope)
-            << Scope->getSourceRange();
-      else if (!isValidSyncScopeValue(Result.getZExtValue()))
-        Diag(Scope->getLocStart(), diag::err_atomic_op_has_invalid_synch_scope)
-            << Scope->getSourceRange();
-    } else {
-      Scope = IntegerLiteral::Create(
-          Context,
-          llvm::APInt(Context.getTypeSize(Context.IntTy),
-                      static_cast<unsigned>(SyncScope::OpenCLAllSVMDevices)),
-          Context.IntTy, SourceLocation());
-    }
-  }
-
   // Permute the arguments into a 'consistent' order.
   SmallVector<Expr*, 5> SubExprs;
   SubExprs.push_back(Ptr);
   switch (Form) {
   case Init:
     // Note, AtomicExpr::getVal1() has a special case for this atomic.
     SubExprs.push_back(TheCall->getArg(1)); // Val1
     break;
   case Load:
     SubExprs.push_back(TheCall->getArg(1)); // Order
-    SubExprs.push_back(Scope);              // Scope
     break;
   case LoadCopy:
   case Copy:
   case Arithmetic:
   case Xchg:
     SubExprs.push_back(TheCall->getArg(2)); // Order
-    SubExprs.push_back(Scope);              // Scope
     SubExprs.push_back(TheCall->getArg(1)); // Val1
     break;
   case GNUXchg:
     // Note, AtomicExpr::getVal2() has a special case for this atomic.
     SubExprs.push_back(TheCall->getArg(3)); // Order
-    SubExprs.push_back(Scope);              // Scope
     SubExprs.push_back(TheCall->getArg(1)); // Val1
     SubExprs.push_back(TheCall->getArg(2)); // Val2
     break;
   case C11CmpXchg:
     SubExprs.push_back(TheCall->getArg(3)); // Order
-    SubExprs.push_back(Scope);              // Scope
     SubExprs.push_back(TheCall->getArg(1)); // Val1
     SubExprs.push_back(TheCall->getArg(4)); // OrderFail
     SubExprs.push_back(TheCall->getArg(2)); // Val2
     break;
   case GNUCmpXchg:
     SubExprs.push_back(TheCall->getArg(4)); // Order
-    SubExprs.push_back(Scope);              // Scope
     SubExprs.push_back(TheCall->getArg(1)); // Val1
     SubExprs.push_back(TheCall->getArg(5)); // OrderFail
     SubExprs.push_back(TheCall->getArg(2)); // Val2
     SubExprs.push_back(TheCall->getArg(3)); // Weak
     break;
   }
 
   if (SubExprs.size() >= 2 && Form != Init) {
     llvm::APSInt Result(32);
     if (SubExprs[1]->isIntegerConstantExpr(Result, Context) &&
         !isValidOrderingForOp(Result.getSExtValue(), Op))
       Diag(SubExprs[1]->getLocStart(),
            diag::warn_atomic_op_has_invalid_memory_order)
           << SubExprs[1]->getSourceRange();
   }
 
+  if (auto ScopeModel = AtomicExpr::getScopeModel(Op)) {
+    auto *Scope = TheCall->getArg(TheCall->getNumArgs() - 1);
+    llvm::APSInt Result(32);
+    if (Scope->isIntegerConstantExpr(Result, Context) &&
+        !ScopeModel->isValid(Result.getZExtValue())) {
+      Diag(Scope->getLocStart(), diag::err_atomic_op_has_invalid_synch_scope)
+          << Scope->getSourceRange();
+    }
+    SubExprs.push_back(Scope);
+  }
+
   AtomicExpr *AE = new (Context) AtomicExpr(TheCall->getCallee()->getLocStart(),
                                             SubExprs, ResultType, Op,
                                             TheCall->getRParenLoc());
   
   if ((Op == AtomicExpr::AO__c11_atomic_load ||
        Op == AtomicExpr::AO__c11_atomic_store ||
        Op == AtomicExpr::AO__opencl_atomic_load ||
        Op == AtomicExpr::AO__opencl_atomic_store ) &&

cfe/trunk/test/CodeGenOpenCL/atomic-ops-libcall.cl

 // RUN: %clang_cc1 < %s -cl-std=CL2.0 -finclude-default-header -triple spir64 -emit-llvm | FileCheck -check-prefix=SPIR %s
 // RUN: %clang_cc1 < %s -cl-std=CL2.0 -finclude-default-header -triple armv5e-none-linux-gnueabi -emit-llvm | FileCheck -check-prefix=ARM %s
 
-void f(atomic_int *i, atomic_uint *ui, int cmp) {
+void f(atomic_int *i, atomic_uint *ui, int cmp, int order, int scope) {
   int x;
   // SPIR: {{%[^ ]*}} = call i32 @__opencl_atomic_load_4(i8 addrspace(4)* {{%[0-9]+}}, i32 5, i32 1)
   // ARM: {{%[^ ]*}} = call i32 @__opencl_atomic_load_4(i8* {{%[0-9]+}}, i32 5, i32 1)
   x = __opencl_atomic_load(i, memory_order_seq_cst, memory_scope_work_group);
   // SPIR: call void @__opencl_atomic_store_4(i8 addrspace(4)* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 1)
   // ARM: call void @__opencl_atomic_store_4(i8* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 1)
   __opencl_atomic_store(i, 1, memory_order_seq_cst, memory_scope_work_group);
   // SPIR: {{%[^ ]*}} = call i32 @__opencl_atomic_fetch_add_4(i8 addrspace(4)* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 1)
void f(atomic_int *i, atomic_uint *ui, int cmp, int order, int scope) {
   x = __opencl_atomic_compare_exchange_weak(i, &cmp, 1, memory_order_seq_cst, memory_order_seq_cst, memory_scope_device);
   // SPIR: {{%[^ ]*}} = call zeroext i1 @__opencl_atomic_compare_exchange_4(i8 addrspace(4)* {{%[0-9]+}}, i8 addrspace(4)* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 5, i32 3)
   // ARM: {{%[^ ]*}} = call zeroext i1 @__opencl_atomic_compare_exchange_4(i8* {{%[0-9]+}}, i8* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 5, i32 3)
   x = __opencl_atomic_compare_exchange_weak(i, &cmp, 1, memory_order_seq_cst, memory_order_seq_cst, memory_scope_all_svm_devices);
 #ifdef cl_khr_subgroups
   // SPIR: {{%[^ ]*}} = call zeroext i1 @__opencl_atomic_compare_exchange_4(i8 addrspace(4)* {{%[0-9]+}}, i8 addrspace(4)* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 5, i32 5, i32 4)
   x = __opencl_atomic_compare_exchange_weak(i, &cmp, 1, memory_order_seq_cst, memory_order_seq_cst, memory_scope_sub_group);
 #endif
+  // SPIR: {{%[^ ]*}} = call zeroext i1 @__opencl_atomic_compare_exchange_4(i8 addrspace(4)* {{%[0-9]+}}, i8 addrspace(4)* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+  // ARM: {{%[^ ]*}} = call zeroext i1 @__opencl_atomic_compare_exchange_4(i8* {{%[0-9]+}}, i8* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+  x = __opencl_atomic_compare_exchange_weak(i, &cmp, 1, order, order, scope);
 }

cfe/trunk/test/CodeGenOpenCL/atomic-ops.cl

bool fi4(atomic_int *i) {
   // CHECK: [[OLD:%[.0-9A-Z_a-z]+]] = extractvalue { i32, i1 } [[PAIR]], 0
   // CHECK: [[CMP:%[.0-9A-Z_a-z]+]] = extractvalue { i32, i1 } [[PAIR]], 1
   // CHECK: br i1 [[CMP]], label %[[STORE_EXPECTED:[.0-9A-Z_a-z]+]], label %[[CONTINUE:[.0-9A-Z_a-z]+]]
   // CHECK: store i32 [[OLD]]
   int cmp = 0;
   return __opencl_atomic_compare_exchange_strong(i, &cmp, 1, memory_order_acquire, memory_order_acquire, memory_scope_work_group);
 }
 
+void fi5(atomic_int *i, int scope) {
+  // CHECK-LABEL: @fi5
+  // CHECK: switch i32 %{{.*}}, label %opencl_allsvmdevices [
+  // CHECK-NEXT: i32 1, label %opencl_workgroup
+  // CHECK-NEXT: i32 2, label %opencl_device
+  // CHECK-NEXT: i32 4, label %opencl_subgroup
+  // CHECK-NEXT: ]
+  // CHECK: opencl_workgroup:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} syncscope("workgroup") seq_cst
+  // CHECK: br label %atomic.scope.continue
+  // CHECK: opencl_device:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} syncscope("agent") seq_cst
+  // CHECK: br label %atomic.scope.continue
+  // CHECK: opencl_allsvmdevices:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} seq_cst, align 4
+  // CHECK: br label %atomic.scope.continue
+  // CHECK: opencl_subgroup:
+  // CHECK: %5 = load atomic i32, i32 addrspace(4)* %0 syncscope("subgroup") seq_cst, align 4
+  // CHECK: br label %atomic.scope.continue
+  // CHECK: atomic.scope.continue:
+  int x = __opencl_atomic_load(i, memory_order_seq_cst, scope);
+}
+
+void fi6(atomic_int *i, int order, int scope) {
+  // CHECK-LABEL: @fi6
+  // CHECK: switch i32 %{{.*}}, label %monotonic [
+  // CHECK-NEXT: i32 1, label %acquire
+  // CHECK-NEXT: i32 2, label %acquire
+  // CHECK-NEXT: i32 5, label %seqcst
+  // CHECK-NEXT: ]
+  // CHECK: monotonic:
+  // CHECK: switch i32 %{{.*}}, label %[[MON_ALL:.*]] [
+  // CHECK-NEXT: i32 1, label %[[MON_WG:.*]]
+  // CHECK-NEXT: i32 2, label %[[MON_DEV:.*]]
+  // CHECK-NEXT: i32 4, label %[[MON_SUB:.*]]
+  // CHECK-NEXT: ]
+  // CHECK: acquire:
+  // CHECK: switch i32 %{{.*}}, label %[[ACQ_ALL:.*]] [
+  // CHECK-NEXT: i32 1, label %[[ACQ_WG:.*]]
+  // CHECK-NEXT: i32 2, label %[[ACQ_DEV:.*]]
+  // CHECK-NEXT: i32 4, label %[[ACQ_SUB:.*]]
+  // CHECK-NEXT: ]
+  // CHECK: seqcst:
+  // CHECK: switch i32 %2, label %[[SEQ_ALL:.*]] [
+  // CHECK-NEXT: i32 1, label %[[SEQ_WG:.*]]
+  // CHECK-NEXT: i32 2, label %[[SEQ_DEV:.*]]
+  // CHECK-NEXT: i32 4, label %[[SEQ_SUB:.*]]
+  // CHECK-NEXT: ]
+  // CHECK: [[MON_WG]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} syncscope("workgroup") monotonic
+  // CHECK: [[MON_DEV]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} syncscope("agent") monotonic
+  // CHECK: [[MON_ALL]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} monotonic
+  // CHECK: [[MON_SUB]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} syncscope("subgroup") monotonic
+  // CHECK: [[ACQ_WG]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} syncscope("workgroup") acquire
+  // CHECK: [[ACQ_DEV]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} syncscope("agent") acquire
+  // CHECK: [[ACQ_ALL]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} acquire
+  // CHECK: [[ACQ_SUB]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} syncscope("subgroup") acquire
+  // CHECK: [[SEQ_WG]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} syncscope("workgroup") seq_cst
+  // CHECK: [[SEQ_DEV]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} syncscope("agent") seq_cst
+  // CHECK: [[SEQ_ALL]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} seq_cst
+  // CHECK: [[SEQ_SUB]]:
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{.*}} syncscope("subgroup") seq_cst
+  int x = __opencl_atomic_load(i, order, scope);
+}
+
 float ff1(global atomic_float *d) {
   // CHECK-LABEL: @ff1
   // CHECK: load atomic i32, i32 addrspace(1)* {{.*}} syncscope("workgroup") monotonic
   return __opencl_atomic_load(d, memory_order_relaxed, memory_scope_work_group);
 }
 
 void ff2(atomic_float *d) {
   // CHECK-LABEL: @ff2

cfe/trunk/test/SemaOpenCL/atomic-ops.cl

 // RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -verify -fsyntax-only -triple=spir64
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -finclude-default-header -verify -fsyntax-only -triple=amdgcn-amdhsa-amd-opencl
 
 // Basic parsing/Sema tests for __opencl_atomic_*
 
 #pragma OPENCL EXTENSION cl_khr_int64_base_atomics : enable
 #pragma OPENCL EXTENSION cl_khr_int64_extended_atomics : enable
 
 struct S { char c[3]; };
 
 char i8;
 short i16;
 int i32;
 int8 i64;
 
 atomic_int gn;
-
 void f(atomic_int *i, const atomic_int *ci,
        atomic_intptr_t *p, atomic_float *d,
        int *I, const int *CI,
        intptr_t *P, float *D, struct S *s1, struct S *s2,
        global atomic_int *i_g, local atomic_int *i_l, private atomic_int *i_p,
        constant atomic_int *i_c) {
   __opencl_atomic_init(I, 5); // expected-error {{address argument to atomic operation must be a pointer to _Atomic type ('__generic int *' invalid)}}
   __opencl_atomic_init(ci, 5); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const __generic atomic_int *' (aka 'const __generic _Atomic(int) *') invalid)}}
void f(atomic_int *i, const atomic_int *ci,
   __opencl_atomic_store(ci, 0, memory_order_release, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const __generic atomic_int *' (aka 'const __generic _Atomic(int) *') invalid)}}
   __opencl_atomic_load(ci, memory_order_acquire, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const __generic atomic_int *' (aka 'const __generic _Atomic(int) *') invalid)}}
 
   __opencl_atomic_init(&gn, 456);
   __opencl_atomic_init(&gn, (void*)0); // expected-warning{{incompatible pointer to integer conversion passing '__generic void *' to parameter of type 'int'}}
 }
 
 void memory_checks(atomic_int *Ap, int *p, int val) {
+  // non-integer memory order argument is casted to integer type.
+  (void)__opencl_atomic_load(Ap, 1.0f, memory_scope_work_group);
+  float forder;
+  (void)__opencl_atomic_load(Ap, forder, memory_scope_work_group);
+  struct S s;
+  (void)__opencl_atomic_load(Ap, s, memory_scope_work_group); // expected-error {{passing 'struct S' to parameter of incompatible type 'int'}}
+
   (void)__opencl_atomic_load(Ap, memory_order_relaxed, memory_scope_work_group);
   (void)__opencl_atomic_load(Ap, memory_order_acquire, memory_scope_work_group);
   (void)__opencl_atomic_load(Ap, memory_order_consume, memory_scope_work_group); // expected-error {{use of undeclared identifier 'memory_order_consume'}}
   (void)__opencl_atomic_load(Ap, memory_order_release, memory_scope_work_group); // expected-warning {{memory order argument to atomic operation is invalid}}
   (void)__opencl_atomic_load(Ap, memory_order_acq_rel, memory_scope_work_group); // expected-warning {{memory order argument to atomic operation is invalid}}
   (void)__opencl_atomic_load(Ap, memory_order_seq_cst, memory_scope_work_group);
 
   (void)__opencl_atomic_store(Ap, val, memory_order_relaxed, memory_scope_work_group);

 }
 
 void synchscope_checks(atomic_int *Ap, int scope) {
   (void)__opencl_atomic_load(Ap, memory_order_relaxed, memory_scope_work_item); // expected-error{{synchronization scope argument to atomic operation is invalid}}
   (void)__opencl_atomic_load(Ap, memory_order_relaxed, memory_scope_work_group);
   (void)__opencl_atomic_load(Ap, memory_order_relaxed, memory_scope_device);
   (void)__opencl_atomic_load(Ap, memory_order_relaxed, memory_scope_all_svm_devices);
   (void)__opencl_atomic_load(Ap, memory_order_relaxed, memory_scope_sub_group);
-  (void)__opencl_atomic_load(Ap, memory_order_relaxed, scope); // expected-error{{non-constant synchronization scope argument to atomic operation is not supported}}
+  (void)__opencl_atomic_load(Ap, memory_order_relaxed, scope);
   (void)__opencl_atomic_load(Ap, memory_order_relaxed, 10);    //expected-error{{synchronization scope argument to atomic operation is invalid}}
+
+  // non-integer memory scope is casted to integer type.
+  float fscope;
+  (void)__opencl_atomic_load(Ap, memory_order_relaxed, 1.0f);
+  (void)__opencl_atomic_load(Ap, memory_order_relaxed, fscope);
+  struct S s;
+  (void)__opencl_atomic_load(Ap, memory_order_relaxed, s); //expected-error{{passing 'struct S' to parameter of incompatible type 'int'}}
 }
 
 void nullPointerWarning(atomic_int *Ap, int *p, int val) {
   // The 'expected' pointer shouldn't be NULL.
   (void)__opencl_atomic_compare_exchange_strong(Ap, NULL, val, memory_order_relaxed, memory_order_relaxed, memory_scope_work_group); // expected-warning {{null passed to a callee that requires a non-null argument}}
 }