Add OpenCL 2.0 atomic builtin functions as Clang builtin

[yaxunl]

Summary

OpenCL 2.0 atomic builtin functions have a scope argument which is ideally represented as synchronization scope argument in LLVM atomic instructions.

Clang supports translating Clang atomic builtin functions to LLVM atomic instructions. However it currently does not support synchronization scope of LLVM atomic instructions. Without this, users have to use LLVM assembly code to implement OpenCL atomic builtin functions.

This patch adds OpenCL 2.0 atomic builtin functions as Clang builtin functions, which supports generating LLVM atomic instructions with synchronization scope operand.

Currently only constant memory scope argument is supported. Support of non-constant memory scope argument will be added later.

[yaxunl]

Fix typos and formatting issues.

[rjmccall]

This patch changes the language design of the atomic builtins, which is outside the normal scope of patch review. You need to post an RFC to cfe-dev. I've gone ahead and made some material comments, but the concept itself needs debate.

Your proposed language design exposes LLVM internals (the specific values used by llvm::SynchronizationScope) directly to users; that is also unacceptable.

lib/AST/Expr.cpp

3917

Add an extra newline here, please, to be consistent with the other cases in the switch.

lib/CodeGen/CGAtomic.cpp

1033

It is not acceptable to test this exclusively with an assertion; you need to be checking that the argument is an integer constant expression in Sema.

lib/Sema/SemaChecking.cpp

2922

1 is a magic number here.

[yaxunl]

In D28691#648267, @rjmccall wrote:

This patch changes the language design of the atomic builtins, which is outside the normal scope of patch review. You need to post an RFC to cfe-dev. I've gone ahead and made some material comments, but the concept itself needs debate.

Your proposed language design exposes LLVM internals (the specific values used by llvm::SynchronizationScope) directly to users; that is also unacceptable.

I sent an RFC to cfe-dev.

For the synchronization scope, I will add Clang's enums for synchronization scopes in a similar fashion as memory order. Different languages can have their own specific synchronization scopes. Then clang translates them to synchronization scopes used by LLVM atomic instructions.

lib/CodeGen/CGAtomic.cpp

1033

Will add diagnostics to Sema.

lib/Sema/SemaChecking.cpp

2922

Will change that to enum.

[yaxunl]

The link for the RFC:
http://lists.llvm.org/pipermail/cfe-dev/2017-January/052304.html

[yaxunl]

Add __opencl_atomic_ builtins.

[rjmccall]

Looks like you haven't introduced proper constants in the header for scopes.

docs/LanguageExtensions.rst

1855 ↗

(On Diff #101955)

1. "an", not "in".
2. There's no need to put "explicit" in `` quotes.
3. The prefix you're using is "__opencl_", with only one trailing underscore.

lib/CodeGen/CGAtomic.cpp

724

This is equivalent to checking for the two AO kinds, right? Probably better to just do that.

lib/CodeGen/CGExpr.cpp

50 ↗

(On Diff #101955)

I think it would be better to keep this function simple and just add the cast outside in the two places you need it.

Also, please remember to use the target hook instead of using an addrspacecast directly.

lib/Sema/SemaChecking.cpp

3046

This clause now covers the scope argument as well.

3060

You need to check that is a constant expression in the correct range, and you should add a test for that.

3062

Please document the meaning of 1.

[yaxunl]

Revised by John's comments.

[Anastasia]

include/clang/Basic/Builtins.def

713 ↗

(On Diff #102374)

What about min/max? I believe they will need to have the scope too.

include/clang/Basic/DiagnosticSemaKinds.td

6956 ↗

(On Diff #102374)

Btw, is this disallowed by the spec? Can't find anything relevant.

lib/CodeGen/CGAtomic.cpp

666

Seems short enough to introduce an extra variable here. :)

693

The same here... not sure adding an extra variable is helping here. :)

852

Formatting seems to be a bit odd here...

1034

Variable name doesn't follow the style.

1034

could we avoid using C style cast?

lib/Sema/SemaChecking.cpp

3146

formatting seems odd.

test/CodeGenOpenCL/atomic-ops-libcall.cl

1 ↗

(On Diff #102374)

GEN4 -> SPIR

2 ↗

(On Diff #102374)

GEN0 -> AMDGPU

4 ↗

(On Diff #102374)

Could we use different scopes?

test/CodeGenOpenCL/atomic-ops.cl

8

why do we need this?

16

I think we could use different scope types all over...

33

do we have an addrspacecast before?

test/SemaOpenCL/atomic-ops.cl

22 ↗

(On Diff #102374)

could we have the full error for consistency?

30 ↗

(On Diff #102374)

could we also test with constant AS? Also I would generally improve testing wrt address spaces...

[yaxunl]

include/clang/Basic/Builtins.def

713 ↗

(On Diff #102374)

They are not 2.0 atomic builtin functions. They can be implemented as library functions through 2.0 atomic builtin functions.

include/clang/Basic/DiagnosticSemaKinds.td

6956 ↗

(On Diff #102374)

Just temporarily not supported by Clang. Will add support later.

lib/CodeGen/CGAtomic.cpp

666

removed the variable

693

removed the variable

852

this is done by clang-format

1034

will fix

1034

will change to static_cast

lib/Sema/SemaChecking.cpp

3146

this is done by clang-format

test/CodeGenOpenCL/atomic-ops-libcall.cl

1 ↗

(On Diff #102374)

will change

2 ↗

(On Diff #102374)

Actually this triple is armv5e. This test requires a target not supporting atomic instructions. Will change GEN0 -> ARM

4 ↗

(On Diff #102374)

Yes. will add them.

[yaxunl]

test/CodeGenOpenCL/atomic-ops.cl

8

This is to test the builtin works in pch. When generating pch, ALREADY_INCLUDED is undefined, therefore pch will include all functions. When including pch, since ALREADY_INCLUDED is defined through pch, the cl file is empty and function definitions from pch is used for codegen.

16

will add them.

33

No,

test/SemaOpenCL/atomic-ops.cl

22 ↗

(On Diff #102374)

will do

30 ↗

(On Diff #102374)

will add tests for different addr spaces.

[yaxunl]

Rebased to ToT and revised by Anastasia's comments.

[b-sumner]

include/clang/Basic/Builtins.def

713 ↗

(On Diff #102374)

Yes, they are. Please look again at 6.13.11.7.5 in the 2.0 C spec.

[yaxunl]

include/clang/Basic/Builtins.def

713 ↗

(On Diff #102374)

sorry I thought they are just some atomic extensions since C++11 atomic builtins do not have those. I will add them.

[kzhuravl]

Seems like SyncScope.h file is missing?

[b-sumner]

Can we drop the "opencl" part of the name and use something like __scoped_atomic_*? Also, it may not make sense to support non-constant scope here since we can't predict what other scopes may be added by other languages in the future.

[yaxunl]

In D28691#820375, @kzhuravl wrote:

Seems like SyncScope.h file is missing?

Right. I will add it.

[yaxunl]

Add min/max and missing file.

[yaxunl]

In D28691#820466, @b-sumner wrote:

Can we drop the "opencl" part of the name and use something like __scoped_atomic_*? Also, it may not make sense to support non-constant scope here since we can't predict what other scopes may be added by other languages in the future.

we could use the approach of LangAS, i.e. we allow targets to map all language specific scopes to target-specific scope names, since IR only cares about scope names, which are target specific. And this is what the current implementation does.

I have no objection to use the __scoped_atomic_ name. It is more general and extensible. John/Anastasia, any comments? Thanks.

[rjmccall]

In D28691#820489, @yaxunl wrote:

In D28691#820466, @b-sumner wrote:

Can we drop the "opencl" part of the name and use something like __scoped_atomic_*? Also, it may not make sense to support non-constant scope here since we can't predict what other scopes may be added by other languages in the future.

we could use the approach of LangAS, i.e. we allow targets to map all language specific scopes to target-specific scope names, since IR only cares about scope names, which are target specific. And this is what the current implementation does.

I have no objection to use the __scoped_atomic_ name. It is more general and extensible. John/Anastasia, any comments? Thanks.

I think I would prefer __opencl_atomic_* until we have some evidence that this concept is more general than just OpenCL.

[b-sumner]

In D28691#820526, @rjmccall wrote:

In D28691#820489, @yaxunl wrote:

In D28691#820466, @b-sumner wrote:

Can we drop the "opencl" part of the name and use something like __scoped_atomic_*? Also, it may not make sense to support non-constant scope here since we can't predict what other scopes may be added by other languages in the future.

we could use the approach of LangAS, i.e. we allow targets to map all language specific scopes to target-specific scope names, since IR only cares about scope names, which are target specific. And this is what the current implementation does.

I have no objection to use the __scoped_atomic_ name. It is more general and extensible. John/Anastasia, any comments? Thanks.

I think I would prefer __opencl_atomic_* until we have some evidence that this concept is more general than just OpenCL.

There are other languages for heterogeneous compute that have scopes, although not exposed quite as explicitly as OpenCL. For example AMD's "HC" language. And any language making use of clang and targeting SPIR-V would likely use these builtins. I think a more generic prefix is appropriate, and "scoped" tells us exactly when these are needed.

[rjmccall]

In D28691#820541, @b-sumner wrote:

There are other languages for heterogeneous compute that have scopes, although not exposed quite as explicitly as OpenCL. For example AMD's "HC" language. And any language making use of clang and targeting SPIR-V would likely use these builtins. I think a more generic prefix is appropriate, and "scoped" tells us exactly when these are needed.

But would those languages use the same language design for these scopes as OpenCL if they did expose them, as opposed to some more elaborate scoping specification? My objection is not that the concept is inherently OpenCL-specific, it's that the presentation in the language might be inherently OpenCL-specific, which makes staying in the opencl namespace is prudent.

[b-sumner]

In D28691#820595, @rjmccall wrote:

In D28691#820541, @b-sumner wrote:

There are other languages for heterogeneous compute that have scopes, although not exposed quite as explicitly as OpenCL. For example AMD's "HC" language. And any language making use of clang and targeting SPIR-V would likely use these builtins. I think a more generic prefix is appropriate, and "scoped" tells us exactly when these are needed.

But would those languages use the same language design for these scopes as OpenCL if they did expose them, as opposed to some more elaborate scoping specification? My objection is not that the concept is inherently OpenCL-specific, it's that the presentation in the language might be inherently OpenCL-specific, which makes staying in the opencl namespace is prudent.

Are you envisioning a language far enough from C/C++ that a standard library or header would not be able to map a scoped atomic operation into a call to one of these new builtins? Would we expect more of such languages than languages that would do such a mapping?

[rjmccall]

In D28691#820641, @b-sumner wrote:

In D28691#820595, @rjmccall wrote:

In D28691#820541, @b-sumner wrote:

There are other languages for heterogeneous compute that have scopes, although not exposed quite as explicitly as OpenCL. For example AMD's "HC" language. And any language making use of clang and targeting SPIR-V would likely use these builtins. I think a more generic prefix is appropriate, and "scoped" tells us exactly when these are needed.

But would those languages use the same language design for these scopes as OpenCL if they did expose them, as opposed to some more elaborate scoping specification? My objection is not that the concept is inherently OpenCL-specific, it's that the presentation in the language might be inherently OpenCL-specific, which makes staying in the opencl namespace is prudent.

Are you envisioning a language far enough from C/C++ that a standard library or header would not be able to map a scoped atomic operation into a call to one of these new builtins? Would we expect more of such languages than languages that would do such a mapping?

If you're using Clang as a frontend for your language, it must be similar enough to C to call a builtin function. That's not at issue. The central question here is whether these builtins are meaningfully general outside of OpenCL. The concept of heterogenous computation is certainly not specific to OpenCL; however, these builtins are defined in terms of scopes — "work item", "work group", "device", and "all svm devices" — which, it seems to me, are basically only defined by reference to the OpenCL architecture. A different heterogenous compute environment might reasonably formalize scopes in a completely different way; for example, it might wish to be more explicit about exactly which peers / devices to synchronize with.

SPIR is explicitly defined on top of the OpenCL model. Users should be able to use OpenCL builtins when targeting it. That does not make those builtins more general than OpenCL.

John.

[t-tye]

include/clang/Basic/Builtins.def

717 ↗

(On Diff #108127)

Will the OpenCL 2.0 memory fences also be supported which also have a memory order and memory scope?

include/clang/Basic/SyncScope.h

25–29 ↗

(On Diff #108127)

Since the builtins are being named as __opencl then should these also be named as opencl_ since they are the memory scopes for OpenCL using the OpenCL numeric values?

If another language wants to use memory scopes, would it then add its own langx_* names in a similar way that is done for address spaces where the LangAS enumeration type has values for each distinct language. Each target is then responsible for mapping each language scope to the appropriate target specific scope as is done for address spaces?

If so then the builtins are really supporting the concept of memory scopes and are not language specific as this enumeration can support multiple languages in the same way as the LangAS enumeration supports multiple languages. If so the builtins would be better named to reflect this as @b-sumner suggested.

[Anastasia]

In D28691#820684, @rjmccall wrote:

In D28691#820641, @b-sumner wrote:

In D28691#820595, @rjmccall wrote:

In D28691#820541, @b-sumner wrote:

There are other languages for heterogeneous compute that have scopes, although not exposed quite as explicitly as OpenCL. For example AMD's "HC" language. And any language making use of clang and targeting SPIR-V would likely use these builtins. I think a more generic prefix is appropriate, and "scoped" tells us exactly when these are needed.

But would those languages use the same language design for these scopes as OpenCL if they did expose them, as opposed to some more elaborate scoping specification? My objection is not that the concept is inherently OpenCL-specific, it's that the presentation in the language might be inherently OpenCL-specific, which makes staying in the opencl namespace is prudent.

Are you envisioning a language far enough from C/C++ that a standard library or header would not be able to map a scoped atomic operation into a call to one of these new builtins? Would we expect more of such languages than languages that would do such a mapping?

If you're using Clang as a frontend for your language, it must be similar enough to C to call a builtin function. That's not at issue. The central question here is whether these builtins are meaningfully general outside of OpenCL. The concept of heterogenous computation is certainly not specific to OpenCL; however, these builtins are defined in terms of scopes — "work item", "work group", "device", and "all svm devices" — which, it seems to me, are basically only defined by reference to the OpenCL architecture. A different heterogenous compute environment might reasonably formalize scopes in a completely different way; for example, it might wish to be more explicit about exactly which peers / devices to synchronize with.

SPIR is explicitly defined on top of the OpenCL model. Users should be able to use OpenCL builtins when targeting it. That does not make those builtins more general than OpenCL.

John.

The scope concept in OpenCL is fairly generic. And the builtins just take one extra argument on top of the existing C11 builtin style. The OpenCL scopes have of course specific meaning to the OpenCL model, but there is nothing preventing other uses of the scope feature. As far as I understand atomic scope implementation in LLVM is fairly generic wrt scope types and it is intended for broader functionality than just OpenCL. So I would vote for having this as generic as possible in Clang too even though I don't think name prefix __opencl is preventing from using this feature in other languages unless we would disallow the builtins in the other language dialects. Which is not the case with the current patch because the builtins are added as ATOMIC_BUILTIN and not LANGBUILTIN. We have for example used C11 builtin in OpenCL already. So other cases are possible too.

include/clang/Basic/SyncScope.h

25–29 ↗

(On Diff #108127)

We generally prefix the names of OpenCL specific implementations. So perhaps we should do some renaming here in case we don't intend this to be generic implementation.

lib/Sema/SemaChecking.cpp

3160

Could we merge this and the line after please.

[rjmccall]

In D28691#823810, @Anastasia wrote:

In D28691#820684, @rjmccall wrote:

In D28691#820641, @b-sumner wrote:

In D28691#820595, @rjmccall wrote:

In D28691#820541, @b-sumner wrote:

There are other languages for heterogeneous compute that have scopes, although not exposed quite as explicitly as OpenCL. For example AMD's "HC" language. And any language making use of clang and targeting SPIR-V would likely use these builtins. I think a more generic prefix is appropriate, and "scoped" tells us exactly when these are needed.

But would those languages use the same language design for these scopes as OpenCL if they did expose them, as opposed to some more elaborate scoping specification? My objection is not that the concept is inherently OpenCL-specific, it's that the presentation in the language might be inherently OpenCL-specific, which makes staying in the opencl namespace is prudent.

Are you envisioning a language far enough from C/C++ that a standard library or header would not be able to map a scoped atomic operation into a call to one of these new builtins? Would we expect more of such languages than languages that would do such a mapping?

If you're using Clang as a frontend for your language, it must be similar enough to C to call a builtin function. That's not at issue. The central question here is whether these builtins are meaningfully general outside of OpenCL. The concept of heterogenous computation is certainly not specific to OpenCL; however, these builtins are defined in terms of scopes — "work item", "work group", "device", and "all svm devices" — which, it seems to me, are basically only defined by reference to the OpenCL architecture. A different heterogenous compute environment might reasonably formalize scopes in a completely different way; for example, it might wish to be more explicit about exactly which peers / devices to synchronize with.

SPIR is explicitly defined on top of the OpenCL model. Users should be able to use OpenCL builtins when targeting it. That does not make those builtins more general than OpenCL.

John.

The scope concept in OpenCL is fairly generic. And the builtins just take one extra argument on top of the existing C11 builtin style. The OpenCL scopes have of course specific meaning to the OpenCL model, but there is nothing preventing other uses of the scope feature.

Yes, it is possible that some other language could introduce exactly the same scope-atomics concept only with a slightly different enumeration of scopes. But then we really shouldn't allow the OpenCL scopes to be used in that language, which means there would still not be a language-independent way of using these builtins.

As far as I understand atomic scope implementation in LLVM is fairly generic wrt scope types and it is intended for broader functionality than just OpenCL.

In some ways this is reasoning the wrong way around. I am not deeply informed about heterogenous computing, so I am happy to accept that llvm::SynchronizationScope is well-designed for our current needs. But LLVM's representation is, by design, ultimately just an implementation detail and can be easily updated — it's just a matter of changing a few calls and adding an upgrade path to the bitcode loader, exactly as we did when we introduced llvm::SynchronizationScope. That is not true of source language features, even builtins; their design is a contract with programmers, and the bar is substantially higher.

We do not have a compelling reason to claim that these are generally useful, so we should not. They should stay namespaced and be flagged as language-specific builtins.

John.

[rjmccall]

include/clang/Basic/SyncScope.h

1 ↗

(On Diff #108127)

Capitalization.

20 ↗

(On Diff #108127)

LLVM uses this namespace pattern in code that predates the addition of scoped enums to C++. Those days are behind us; we should just use a scoped enum.

22 ↗

(On Diff #108127)

It defines the synch scope values used by the atomic builtins and expressions. LLVM's headers define the values used by the instructions.

25–29 ↗

(On Diff #108127)

I agree that we should name the OpenCL-specific ones, like WorkGroup, with an OpenCL prefix.

32 ↗

(On Diff #108127)

This is C++; please just use () instead of (void).

lib/CodeGen/CGAtomic.cpp

502

None of the .getTypePtr() stuff here is necessary.

This function shouldn't really be necessary. I would encourage you to add a getValueType() accessor to AtomicExpr:

QualType AtomicExpr::getValueType() const {
  auto T = getPtr()->getType()->castTo<PointerType>()->getPointeeType();
  if (auto AT = T->getAs<AtomicType>()) {
    return AT->getValueType();
  } else {
    return T;
  }
}

You can then just use E->getValueType()->isSignedIntegerType() and eliminate this helper function.

855

Again you're using getTypePtr() unnecessarily.

The check should be whether the AST-level address spaces match, not whether the lowered address spaces match.

Please pass E->getType() instead of E here.

You should remove the DoIt parameter and just check E->isOpenCL() (where E is the AtomicExpr already in scope).

lib/CodeGen/CGCall.cpp

3911 ↗

(On Diff #108127)

No. Callers should ensure that they've added the right argument type, at least at the level of address spaces.

lib/CodeGen/TargetInfo.h

266 ↗

(On Diff #108127)

Why does this return a StringRef instead of an llvm::SynchronizationScope?

[yaxunl]

Revised by reviewers' comments.

[t-tye]

include/clang/Basic/SyncScope.h

23 ↗

(On Diff #109406)

The OpenCL workitem scope is only used for image fences and does not apply to atomic operations so not sure that it should be in this enumeration which is used only for memory atomics.

lib/CodeGen/TargetInfo.cpp

7554–7555 ↗

(On Diff #109406)

OpenCL only uses workitem for image fences which are not the same as atomic memory fences.

For image fences I don't think it would map to singlethread which is intended for signal handler support to ensure a signal handler has visibility of the updates done by a thread which is more of an optimization barrier. In contrast an image fence may need to flush caches to make the image and vector access paths coherent in a single thread.

Since this patch does not support fences probably want to leave workitem scope out. Current AMDGPU targets do not need to do anything for an OpenCL image fence, but other targets may need to generate an intrinsic since there is no LLVMIR instruction for this.

lib/Headers/opencl-c.h

13145–13150 ↗

(On Diff #109406)

Do these have to have the same values as the SycnScope enumeration? Should that be ensured in a similar way to the memory_order enumeration?

lib/Sema/SemaChecking.cpp

3103

IIRC OpenCL allows the scope to be a runtime value. So will doing this will likely cause failures in conformance?

[rjmccall]

The patch generally looks good, but if you need to handle non-constant scopes, you should submit a new patch to address that.

lib/CodeGen/CGAtomic.cpp

15

You can sink this line and the next.

lib/Sema/SemaChecking.cpp

3103

Ah, if that's true, you'll need to emit a switch in IRGen, the same way we handle non-constant memory orders.

[yaxunl]

include/clang/Basic/Builtins.def

717 ↗

(On Diff #108127)

I am considering supporting it with a separate patch since this patch is already quite large.

include/clang/Basic/SyncScope.h

23 ↗

(On Diff #109406)

You are right. I think we should drop it from this enum for now.

lib/CodeGen/CGAtomic.cpp

15

will do.

lib/CodeGen/CGCall.cpp

3911 ↗

(On Diff #108127)

Will remove.

lib/CodeGen/TargetInfo.cpp

7554–7555 ↗

(On Diff #109406)

Thanks. I will remove this for now.

lib/Headers/opencl-c.h

13145–13150 ↗

(On Diff #109406)

It is desirable to have the same value as SyncScope enumeration, otherwise the library has to do the translation when calling __opencl_atomic_* builtins.

Will do.

lib/Sema/SemaChecking.cpp

3103

Will support it in another patch since this one is already quite large.

3160

will do

[yaxunl]

Revised by Tony's and John's comments.

[yaxunl]

Add assert to make sure pre-defined macros __OPENCL_MEMORY_SCOP_* to be consistent with SyncScope enum.

[rjmccall]

LGTM. I'm fine with the plan to handle potentially non-constant scopes in a follow-up patch.

include/clang/Basic/SyncScope.h

21 ↗

(On Diff #109556)

If the numeric values here were chosen to align with the arguments to some runtime function, that's important to leave as a comment.

lib/Headers/opencl-c.h

13145–13150 ↗

(On Diff #109406)

Since we're defining these builtins ourselves de novo, it's fine to pick argument values that align with what the existing runtime functions expect. Once the builtins are defined and in-use, of course, we cannot subsequently change the builtin values, even if the runtime changes.

[yaxunl]

Add comments to SyncScope.h

[t-tye]

docs/LanguageExtensions.rst

1935 ↗

(On Diff #109591)

Should it also say:

The macros ``__OPENCL_MEMORY_SCOPE_WORK_ITEM``, ``__OPENCL_MEMORY_SCOPE_WORK_GROUP``, ``__OPENCL_MEMORY_SCOPE_DEVICE``, ``__OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES``, and ``__OPENCL_MEMORY_SCOPE_SUB_GROUP`` are provided, with values corresponding to the enumerators of OpenCL's ``memory_scope`` enumeration.

[yaxunl]

docs/LanguageExtensions.rst

1935 ↗

(On Diff #109591)

Thanks. Will do.

[yaxunl]

Added documentation about __OPENCL_MEMORY_SCOPE_* by Tony's comments.

[t-tye]

LGTM

[yaxunl]

include/clang/Basic/SyncScope.h

21 ↗

(On Diff #109556)

I have added the comments and updated the review.. On the Phabricator web page, it may take a bit effort to find it since it is interleaved with reviewers' comments.

[bader]

lib/AST/Expr.cpp

4000–4004

No need in else branch after return:

if (...) {
  return AT->getValueType();
}

return T;

http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

test/SemaOpenCL/atomic-ops.cl

1 ↗

(On Diff #109612)

It's a pity, we have to parse the whole opencl-c.h file to get two enums and one typedef...

[yaxunl]

Ping. Any other comments? Thanks.

lib/AST/Expr.cpp

4000–4004

will fix when committing.

test/SemaOpenCL/atomic-ops.cl

1 ↗

(On Diff #109612)

Since there is change in opencl-c.h, using the header can test that.

[rjmccall]

Still LGTM.

include/clang/AST/Expr.h

 
   friend TrailingObjects;
   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>.
-/// All of these instructions take one primary pointer and at least one memory
-/// order.
+/// All of these instructions take one primary pointer, at least one memory
+/// order, and one memory 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, VAL1, ORDER_FAIL, VAL2, WEAK, END_EXPR };
+  enum { PTR, ORDER, SCOPE, VAL1, ORDER_FAIL, VAL2, WEAK, END_EXPR };
   Stmt* SubExprs[END_EXPR];
   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]);
+  }
   Expr *getVal1() const {
     if (Op == AO__c11_atomic_init)
       return cast<Expr>(SubExprs[ORDER]);
     assert(NumSubExprs > VAL1);
     return cast<Expr>(SubExprs[VAL1]);
   }
   Expr *getOrderFail() const {
     assert(NumSubExprs > ORDER_FAIL);

lib/AST/Expr.cpp

 
     SubExprs[i] = args[i];
   }
 }
 
 unsigned AtomicExpr::getNumSubExprs(AtomicOp Op) {
   switch (Op) {
   case AO__c11_atomic_init:
+    return 2;
   case AO__c11_atomic_load:
   case AO__atomic_load_n:
-    return 2;
+    return 3;
 
   case AO__c11_atomic_store:
   case AO__c11_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__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 3;
+    return 4;
 
   case AO__atomic_exchange:
-    return 4;
+    return 5;
 
   case AO__c11_atomic_compare_exchange_strong:
   case AO__c11_atomic_compare_exchange_weak:
-    return 5;
+    return 6;
 
   case AO__atomic_compare_exchange:
   case AO__atomic_compare_exchange_n:
-    return 6;
+    return 7;
   }
   llvm_unreachable("unknown atomic op");
 }
 
 QualType OMPArraySectionExpr::getBaseOriginalType(const Expr *Base) {
   unsigned ArraySectionCount = 0;
   while (auto *OASE = dyn_cast<OMPArraySectionExpr>(Base->IgnoreParens())) {
     Base = OASE->getBase();

     if (OriginalTy->isAnyPointerType())
       OriginalTy = OriginalTy->getPointeeType();
     else {
       assert (OriginalTy->isArrayType());
       OriginalTy = OriginalTy->castAsArrayTypeUnsafe()->getElementType();
     }
   }
   return OriginalTy;
 }

lib/Basic/Targets.cpp

       hasLDEXPF = true;
     }
 
     resetDataLayout(getTriple().getArch() == llvm::Triple::amdgcn ?
                     DataLayoutStringSI : DataLayoutStringR600);
 
     AddrSpaceMap = &AMDGPUAddrSpaceMap;
     UseAddrSpaceMapMangling = true;
+    MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
   }
 
   uint64_t getPointerWidthV(unsigned AddrSpace) const override {
     if (GPU <= GK_CAYMAN)
       return 32;
 
     switch(AddrSpace) {
       default:

lib/CodeGen/CGAtomic.cpp

 //===--- CGAtomic.cpp - Emit LLVM IR for atomic operations ----------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file contains the code for emitting atomic operations.
 //
 //===----------------------------------------------------------------------===//
 
 #include "CGCall.h"
 #include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Operator.h"
 
bool AtomicInfo::emitMemSetZeroIfNecessary() const {
   return true;
 }
 
 static void emitAtomicCmpXchg(CodeGenFunction &CGF, AtomicExpr *E, bool IsWeak,
                               Address Dest, Address Ptr,
                               Address Val1, Address Val2,
                               uint64_t Size,
                               llvm::AtomicOrdering SuccessOrder,
-                              llvm::AtomicOrdering FailureOrder) {
+                              llvm::AtomicOrdering FailureOrder,
+                              llvm::SynchronizationScope Scope) {
   // Note that cmpxchg doesn't support weak cmpxchg, at least at the moment.
   llvm::Value *Expected = CGF.Builder.CreateLoad(Val1);
   llvm::Value *Desired = CGF.Builder.CreateLoad(Val2);
 
   llvm::AtomicCmpXchgInst *Pair = CGF.Builder.CreateAtomicCmpXchg(
-      Ptr.getPointer(), Expected, Desired, SuccessOrder, FailureOrder);
+      Ptr.getPointer(), Expected, Desired, SuccessOrder, FailureOrder,
+      Scope);
   Pair->setVolatile(E->isVolatile());
   Pair->setWeak(IsWeak);
 
   // Cmp holds the result of the compare-exchange operation: true on success,
   // false on failure.
   llvm::Value *Old = CGF.Builder.CreateExtractValue(Pair, 0);
   llvm::Value *Cmp = CGF.Builder.CreateExtractValue(Pair, 1);
 

 /// Given an ordering required on success, emit all possible cmpxchg
 /// instructions to cope with the provided (but possibly only dynamically known)
 /// FailureOrder.
 static void emitAtomicCmpXchgFailureSet(CodeGenFunction &CGF, AtomicExpr *E,
                                         bool IsWeak, Address Dest, Address Ptr,
                                         Address Val1, Address Val2,
                                         llvm::Value *FailureOrderVal,
                                         uint64_t Size,
-                                        llvm::AtomicOrdering SuccessOrder) {
+                                        llvm::AtomicOrdering SuccessOrder,
+                                        llvm::SynchronizationScope Scope) {
   llvm::AtomicOrdering FailureOrder;
   if (llvm::ConstantInt *FO = dyn_cast<llvm::ConstantInt>(FailureOrderVal)) {
     auto FOS = FO->getSExtValue();
     if (!llvm::isValidAtomicOrderingCABI(FOS))
       FailureOrder = llvm::AtomicOrdering::Monotonic;
     else
       switch ((llvm::AtomicOrderingCABI)FOS) {
       case llvm::AtomicOrderingCABI::relaxed:
else
       }
     if (isStrongerThan(FailureOrder, SuccessOrder)) {
       // Don't assert on undefined behavior "failure argument shall be no
       // stronger than the success argument".
       FailureOrder =
           llvm::AtomicCmpXchgInst::getStrongestFailureOrdering(SuccessOrder);
     }
     emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, SuccessOrder,
-                      FailureOrder);
+                      FailureOrder, Scope);
     return;
   }
 
   // Create all the relevant BB's
   llvm::BasicBlock *MonotonicBB = nullptr, *AcquireBB = nullptr,
                    *SeqCstBB = nullptr;
   MonotonicBB = CGF.createBasicBlock("monotonic_fail", CGF.CurFn);
   if (SuccessOrder != llvm::AtomicOrdering::Monotonic &&
       SuccessOrder != llvm::AtomicOrdering::Release)
     AcquireBB = CGF.createBasicBlock("acquire_fail", CGF.CurFn);
   if (SuccessOrder == llvm::AtomicOrdering::SequentiallyConsistent)
     SeqCstBB = CGF.createBasicBlock("seqcst_fail", CGF.CurFn);
 
   llvm::BasicBlock *ContBB = CGF.createBasicBlock("atomic.continue", CGF.CurFn);
 
   llvm::SwitchInst *SI = CGF.Builder.CreateSwitch(FailureOrderVal, MonotonicBB);
 
   // Emit all the different atomics
 
   // MonotonicBB is arbitrarily chosen as the default case; in practice, this
   // doesn't matter unless someone is crazy enough to use something that
   // doesn't fold to a constant for the ordering.
   CGF.Builder.SetInsertPoint(MonotonicBB);
   emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2,
-                    Size, SuccessOrder, llvm::AtomicOrdering::Monotonic);
+                    Size, SuccessOrder, llvm::AtomicOrdering::Monotonic, Scope);
   CGF.Builder.CreateBr(ContBB);
 
   if (AcquireBB) {
     CGF.Builder.SetInsertPoint(AcquireBB);
     emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2,
-                      Size, SuccessOrder, llvm::AtomicOrdering::Acquire);
+                      Size, SuccessOrder, llvm::AtomicOrdering::Acquire, Scope);
     CGF.Builder.CreateBr(ContBB);
     SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
                 AcquireBB);
     SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::acquire),
                 AcquireBB);
   }
   if (SeqCstBB) {
     CGF.Builder.SetInsertPoint(SeqCstBB);
     emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, SuccessOrder,
-                      llvm::AtomicOrdering::SequentiallyConsistent);
+                      llvm::AtomicOrdering::SequentiallyConsistent, Scope);
     CGF.Builder.CreateBr(ContBB);
     SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::seq_cst),
                 SeqCstBB);
   }
 
   CGF.Builder.SetInsertPoint(ContBB);
 }
 
 static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
                          Address Ptr, Address Val1, Address Val2,
                          llvm::Value *IsWeak, llvm::Value *FailureOrder,
-                         uint64_t Size, llvm::AtomicOrdering Order) {
+                         uint64_t Size, llvm::AtomicOrdering Order,
+                         llvm::SynchronizationScope Scope) {
   llvm::AtomicRMWInst::BinOp Op = llvm::AtomicRMWInst::Add;
   llvm::Instruction::BinaryOps PostOp = (llvm::Instruction::BinaryOps)0;
 
   switch (E->getOp()) {
   case AtomicExpr::AO__c11_atomic_init:
     llvm_unreachable("Already handled!");
 
   case AtomicExpr::AO__c11_atomic_compare_exchange_strong:
     emitAtomicCmpXchgFailureSet(CGF, E, false, Dest, Ptr, Val1, Val2,
-                                FailureOrder, Size, Order);
+                                FailureOrder, Size, Order, Scope);
     return;
   case AtomicExpr::AO__c11_atomic_compare_exchange_weak:
     emitAtomicCmpXchgFailureSet(CGF, E, true, Dest, Ptr, Val1, Val2,
-                                FailureOrder, Size, Order);
+                                FailureOrder, Size, Order, Scope);
     return;
   case AtomicExpr::AO__atomic_compare_exchange:
   case AtomicExpr::AO__atomic_compare_exchange_n: {
     if (llvm::ConstantInt *IsWeakC = dyn_cast<llvm::ConstantInt>(IsWeak)) {
       emitAtomicCmpXchgFailureSet(CGF, E, IsWeakC->getZExtValue(), Dest, Ptr,
-                                  Val1, Val2, FailureOrder, Size, Order);
+                                  Val1, Val2, FailureOrder, Size, Order, Scope);
     } else {
       // Create all the relevant BB's
       llvm::BasicBlock *StrongBB =
           CGF.createBasicBlock("cmpxchg.strong", CGF.CurFn);
       llvm::BasicBlock *WeakBB = CGF.createBasicBlock("cmxchg.weak", CGF.CurFn);
       llvm::BasicBlock *ContBB =
           CGF.createBasicBlock("cmpxchg.continue", CGF.CurFn);
 
       llvm::SwitchInst *SI = CGF.Builder.CreateSwitch(IsWeak, WeakBB);
       SI->addCase(CGF.Builder.getInt1(false), StrongBB);
 
       CGF.Builder.SetInsertPoint(StrongBB);
       emitAtomicCmpXchgFailureSet(CGF, E, false, Dest, Ptr, Val1, Val2,
-                                  FailureOrder, Size, Order);
+                                  FailureOrder, Size, Order, Scope);
       CGF.Builder.CreateBr(ContBB);
 
       CGF.Builder.SetInsertPoint(WeakBB);
       emitAtomicCmpXchgFailureSet(CGF, E, true, Dest, Ptr, Val1, Val2,
-                                  FailureOrder, Size, Order);
+                                  FailureOrder, Size, Order, Scope);
       CGF.Builder.CreateBr(ContBB);
 
       CGF.Builder.SetInsertPoint(ContBB);
     }
     return;
   }
   case AtomicExpr::AO__c11_atomic_load:
   case AtomicExpr::AO__atomic_load_n:
   case AtomicExpr::AO__atomic_load: {
     llvm::LoadInst *Load = CGF.Builder.CreateLoad(Ptr);
-    Load->setAtomic(Order);
+    Load->setAtomic(Order, Scope);
     Load->setVolatile(E->isVolatile());
     CGF.Builder.CreateStore(Load, Dest);
     return;
   }
 
   case AtomicExpr::AO__c11_atomic_store:
   case AtomicExpr::AO__atomic_store:
   case AtomicExpr::AO__atomic_store_n: {
     llvm::Value *LoadVal1 = CGF.Builder.CreateLoad(Val1);
     llvm::StoreInst *Store = CGF.Builder.CreateStore(LoadVal1, Ptr);
-    Store->setAtomic(Order);
+    Store->setAtomic(Order, Scope);
     Store->setVolatile(E->isVolatile());
     return;
   }
 
   case AtomicExpr::AO__c11_atomic_exchange:
   case AtomicExpr::AO__atomic_exchange_n:
   case AtomicExpr::AO__atomic_exchange:
     Op = llvm::AtomicRMWInst::Xchg;
static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
   // Fall through.
   case AtomicExpr::AO__atomic_fetch_nand:
     Op = llvm::AtomicRMWInst::Nand;
     break;
   }
 
   llvm::Value *LoadVal1 = CGF.Builder.CreateLoad(Val1);
   llvm::AtomicRMWInst *RMWI =
-      CGF.Builder.CreateAtomicRMW(Op, Ptr.getPointer(), LoadVal1, Order);
+      CGF.Builder.CreateAtomicRMW(Op, Ptr.getPointer(), LoadVal1, Order, Scope);
   RMWI->setVolatile(E->isVolatile());
 
   // For __atomic_*_fetch operations, perform the operation again to
   // determine the value which was written.
   llvm::Value *Result = RMWI;
   if (PostOp)
     Result = CGF.Builder.CreateBinOp(PostOp, RMWI, LoadVal1);
   if (E->getOp() == AtomicExpr::AO__atomic_nand_fetch)
AddDirectArgument(CodeGenFunction &CGF, CallArgList &Args,
   } else {
     // Non-optimized functions always take a reference.
     Args.add(RValue::get(CGF.EmitCastToVoidPtr(Val)),
                          CGF.getContext().VoidPtrTy);
   }
 }
 
 RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   QualType AtomicTy = E->getPtr()->getType()->getPointeeType();
   QualType MemTy = AtomicTy;
   if (const AtomicType *AT = AtomicTy->getAs<AtomicType>())
     MemTy = AT->getValueType();
   CharUnits sizeChars, alignChars;
   std::tie(sizeChars, alignChars) = getContext().getTypeInfoInChars(AtomicTy);
   uint64_t Size = sizeChars.getQuantity();
   unsigned MaxInlineWidthInBits = getTarget().getMaxAtomicInlineWidth();
   bool UseLibcall = (sizeChars != alignChars ||
                      getContext().toBits(sizeChars) > MaxInlineWidthInBits);
 
   llvm::Value *IsWeak = nullptr, *OrderFail = nullptr;
 
   Address Val1 = Address::invalid();
   Address Val2 = Address::invalid();
   Address Dest = Address::invalid();
   Address Ptr(EmitScalarExpr(E->getPtr()), alignChars);
 
   if (E->getOp() == AtomicExpr::AO__c11_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());
 
   switch (E->getOp()) {
   case AtomicExpr::AO__c11_atomic_init:
     llvm_unreachable("Already handled above with EmitAtomicInit!");
 
   case AtomicExpr::AO__c11_atomic_load:
   case AtomicExpr::AO__atomic_load_n:
     break;
 
   case AtomicExpr::AO__atomic_load:
RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
   case AtomicExpr::AO__atomic_compare_exchange_n:
   case AtomicExpr::AO__atomic_compare_exchange:
     Val1 = EmitPointerWithAlignment(E->getVal1());
     if (E->getOp() == AtomicExpr::AO__atomic_compare_exchange)
       Val2 = EmitPointerWithAlignment(E->getVal2());
     else
       Val2 = EmitValToTemp(*this, E->getVal2());
     OrderFail = EmitScalarExpr(E->getOrderFail());
-    if (E->getNumSubExprs() == 6)
+    if (E->getNumSubExprs() == 7)
       IsWeak = EmitScalarExpr(E->getWeak());
     break;
 
   case AtomicExpr::AO__c11_atomic_fetch_add:
   case AtomicExpr::AO__c11_atomic_fetch_sub:
     if (MemTy->isPointerType()) {
       // For pointer arithmetic, we're required to do a bit of math:
       // adding 1 to an int* is not the same as adding 1 to a uintptr_t.
if (UseLibcall) {
     std::string LibCallName;
     QualType LoweredMemTy =
       MemTy->isPointerType() ? getContext().getIntPtrType() : MemTy;
     QualType RetTy;
     bool HaveRetTy = false;
     llvm::Instruction::BinaryOps PostOp = (llvm::Instruction::BinaryOps)0;
     switch (E->getOp()) {
     case AtomicExpr::AO__c11_atomic_init:
       llvm_unreachable("Already handled!");
 
     // There is only one libcall for compare an exchange, because there is no
     // optimisation benefit possible from a libcall version of a weak compare
     // and exchange.
     // bool __atomic_compare_exchange(size_t size, void *mem, void *expected,
     //                                void *desired, int success, int failure)
     // bool __atomic_compare_exchange_N(T *mem, T *expected, T desired,
     //                                  int success, int failure)
     case AtomicExpr::AO__c11_atomic_compare_exchange_weak:
     case AtomicExpr::AO__c11_atomic_compare_exchange_strong:
     case AtomicExpr::AO__atomic_compare_exchange:
RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
 
   bool IsStore = E->getOp() == AtomicExpr::AO__c11_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__atomic_load ||
                 E->getOp() == AtomicExpr::AO__atomic_load_n;
 
+  assert(isa<llvm::ConstantInt>(Scope) &&
+      "Non-constant synchronization scope not supported");
+  auto sco = (llvm::SynchronizationScope)(
+      cast<llvm::ConstantInt>(Scope)->getZExtValue());
+
   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);
+                     llvm::AtomicOrdering::Monotonic, sco);
         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);
+                     llvm::AtomicOrdering::Acquire, sco);
         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);
+                     llvm::AtomicOrdering::Release, sco);
         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);
+                     llvm::AtomicOrdering::AcquireRelease, sco);
         break;
       case llvm::AtomicOrderingCABI::seq_cst:
         EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail, Size,
-                     llvm::AtomicOrdering::SequentiallyConsistent);
+                     llvm::AtomicOrdering::SequentiallyConsistent, sco);
         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);
+               Size, llvm::AtomicOrdering::Monotonic, sco);
   Builder.CreateBr(ContBB);
   if (!IsStore) {
     Builder.SetInsertPoint(AcquireBB);
     EmitAtomicOp(*this, E, Dest, Ptr, Val1, Val2, IsWeak, OrderFail,
-                 Size, llvm::AtomicOrdering::Acquire);
+                 Size, llvm::AtomicOrdering::Acquire, sco);
     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);
+                 Size, llvm::AtomicOrdering::Release, sco);
     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);
+                 Size, llvm::AtomicOrdering::AcquireRelease, sco);
     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);
+               Size, llvm::AtomicOrdering::SequentiallyConsistent, sco);
   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);

lib/Sema/SemaChecking.cpp

                                          AtomicExpr::AtomicOp Op) {
   CallExpr *TheCall = cast<CallExpr>(TheCallResult.get());
   DeclRefExpr *DRE =cast<DeclRefExpr>(TheCall->getCallee()->IgnoreParenCasts());
 
   // All these operations take one of the following forms:
   enum {
     // C    __c11_atomic_init(A *, C)
     Init,
-    // C    __c11_atomic_load(A *, int)
+    // C    __c11_atomic_load(A *, int, int = 0)
     Load,
-    // void __atomic_load(A *, CP, int)
+    // void __atomic_load(A *, CP, int, int = 0)
     LoadCopy,
-    // void __atomic_store(A *, CP, int)
+    // void __atomic_store(A *, CP, int, int = 0)
     Copy,
-    // C    __c11_atomic_add(A *, M, int)
+    // C    __c11_atomic_add(A *, M, int, int = 0)
     Arithmetic,
-    // C    __atomic_exchange_n(A *, CP, int)
+    // C    __atomic_exchange_n(A *, CP, int, int = 0)
     Xchg,
-    // void __atomic_exchange(A *, C *, CP, int)
+    // void __atomic_exchange(A *, C *, CP, int, int = 0)
     GNUXchg,
-    // bool __c11_atomic_compare_exchange_strong(A *, C *, CP, int, int)
+    // bool __c11_atomic_compare_exchange_strong(A *, C *, CP, int, int, int = 0)
     C11CmpXchg,
-    // bool __atomic_compare_exchange(A *, C *, CP, bool, int, int)
+    // bool __atomic_compare_exchange(A *, C *, CP, bool, int, int, int = 0)
     GNUCmpXchg
   } Form = Init;
-  const unsigned NumArgs[] = { 2, 2, 3, 3, 3, 3, 4, 5, 6 };
+  const unsigned NumArgsMin[] = { 2, 2, 3, 3, 3, 3, 4, 5, 6 };
+  const unsigned NumArgsMax[] = { 2, 3, 4, 4, 4, 4, 5, 6, 7 };
   const unsigned NumVals[] = { 1, 0, 1, 1, 1, 1, 2, 2, 3 };
   // where:
   //   C is an appropriate type,
   //   A is volatile _Atomic(C) for __c11 builtins and is C for GNU builtins,
   //   CP is C for __c11 builtins and GNU _n builtins and is C * otherwise,
   //   M is C if C is an integer, and ptrdiff_t if C is a pointer, and
   //   the int parameters are for orderings.
 

 
   case AtomicExpr::AO__atomic_compare_exchange:
   case AtomicExpr::AO__atomic_compare_exchange_n:
     Form = GNUCmpXchg;
     break;
   }
 
   // Check we have the right number of arguments.
-  if (TheCall->getNumArgs() < NumArgs[Form]) {
+  assert((NumArgsMin[Form] == NumArgsMax[Form] ||
+         NumArgsMin[Form] + 1 == NumArgsMax[Form]) &&
+         "Only one optional argument");
+  if (TheCall->getNumArgs() < NumArgsMin[Form]) {
     Diag(TheCall->getLocEnd(), diag::err_typecheck_call_too_few_args)
-      << 0 << NumArgs[Form] << TheCall->getNumArgs()
+      << 0 << NumArgsMin[Form] << TheCall->getNumArgs()
       << TheCall->getCallee()->getSourceRange();
     return ExprError();
-  } else if (TheCall->getNumArgs() > NumArgs[Form]) {
-    Diag(TheCall->getArg(NumArgs[Form])->getLocStart(),
+  } else if (TheCall->getNumArgs() > NumArgsMax[Form]) {
+    Diag(TheCall->getArg(NumArgsMax[Form])->getLocStart(),
          diag::err_typecheck_call_too_many_args)
-      << 0 << NumArgs[Form] << TheCall->getNumArgs()
+      << 0 << NumArgsMax[Form] << TheCall->getNumArgs()
       << TheCall->getCallee()->getSourceRange();
     return ExprError();
   }
 
   // Inspect the first argument of the atomic operation.
   Expr *Ptr = TheCall->getArg(0);
   ExprResult ConvertedPtr = DefaultFunctionArrayLvalueConversion(Ptr);
   if (ConvertedPtr.isInvalid())

   // arguments are actually passed as pointers.
   QualType ByValType = ValType; // 'CP'
   if (!IsC11 && !IsN)
     ByValType = Ptr->getType();
 
   // The first argument --- the pointer --- has a fixed type; we
   // deduce the types of the rest of the arguments accordingly.  Walk
   // the remaining arguments, converting them to the deduced value type.
-  for (unsigned i = 1; i != NumArgs[Form]; ++i) {
+  for (unsigned i = 1; i != TheCall->getNumArgs(); ++i) {
     QualType Ty;
     if (i < NumVals[Form] + 1) {
       switch (i) {
       case 1:
         // The second argument is the non-atomic operand. For arithmetic, this
         // is always passed by value, and for a compare_exchange it is always
         // passed by address. For the rest, GNU uses by-address and C11 uses
         // by-value.

         Ty = Context.BoolTy;
         break;
       }
     } else {
       // The order(s) are always converted to int.
       Ty = Context.IntTy;
     }
 
       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 (TheCall->getNumArgs() < NumArgsMax[Form]) {
+    Scope = IntegerLiteral::Create(Context,
+      llvm::APInt(Context.getTypeSize(Context.IntTy), (uint64_t) 1),
+      Context.IntTy, SourceLocation());
+  } else {
+    Scope = TheCall->getArg(TheCall->getNumArgs() - 1);
   }
 
   // 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) {

   // Inspect the first argument of the atomic builtin.  This should always be
   // a pointer type, whose element is an integral scalar or pointer type.
   // Because it is a pointer type, we don't have to worry about any implicit
   // casts here.
   // FIXME: We don't allow floating point scalars as input.
   Expr *FirstArg = TheCall->getArg(0);
   ExprResult FirstArgResult = DefaultFunctionArrayLvalueConversion(FirstArg);
   if (FirstArgResult.isInvalid())
     return ExprError();
   FirstArg = FirstArgResult.get();
   TheCall->setArg(0, FirstArg);
 
   const PointerType *pointerType = FirstArg->getType()->getAs<PointerType>();
   if (!pointerType) {
     Diag(DRE->getLocStart(), diag::err_atomic_builtin_must_be_pointer)
       << FirstArg->getType() << FirstArg->getSourceRange();
     return ExprError();
   }
 
   QualType ValType = pointerType->getPointeeType();
   if (!ValType->isIntegerType() && !ValType->isAnyPointerType() &&
       !ValType->isBlockPointerType()) {
     Diag(DRE->getLocStart(), diag::err_atomic_builtin_must_be_pointer_intptr)
       << FirstArg->getType() << FirstArg->getSourceRange();
     return ExprError();
   }
 
   switch (ValType.getObjCLifetime()) {
   case Qualifiers::OCL_None:
   case Qualifiers::OCL_ExplicitNone:
     // okay
     break;
 

     BUILTIN_ROW(__sync_fetch_and_sub),
     BUILTIN_ROW(__sync_fetch_and_or),
     BUILTIN_ROW(__sync_fetch_and_and),
     BUILTIN_ROW(__sync_fetch_and_xor),
     BUILTIN_ROW(__sync_fetch_and_nand),
 
     BUILTIN_ROW(__sync_add_and_fetch),
     BUILTIN_ROW(__sync_sub_and_fetch),
     BUILTIN_ROW(__sync_and_and_fetch),
     BUILTIN_ROW(__sync_or_and_fetch),
     BUILTIN_ROW(__sync_xor_and_fetch),
     BUILTIN_ROW(__sync_nand_and_fetch),
 
     BUILTIN_ROW(__sync_val_compare_and_swap),
     BUILTIN_ROW(__sync_bool_compare_and_swap),
     BUILTIN_ROW(__sync_lock_test_and_set),
     BUILTIN_ROW(__sync_lock_release),

   default: llvm_unreachable("Unknown overloaded atomic builtin!");
   case Builtin::BI__sync_fetch_and_add: 
   case Builtin::BI__sync_fetch_and_add_1:
   case Builtin::BI__sync_fetch_and_add_2:
   case Builtin::BI__sync_fetch_and_add_4:
   case Builtin::BI__sync_fetch_and_add_8:
   case Builtin::BI__sync_fetch_and_add_16:
     BuiltinIndex = 0; 
     break;
       
   case Builtin::BI__sync_fetch_and_sub: 
   case Builtin::BI__sync_fetch_and_sub_1:
   case Builtin::BI__sync_fetch_and_sub_2:
   case Builtin::BI__sync_fetch_and_sub_4:
   case Builtin::BI__sync_fetch_and_sub_8:
   case Builtin::BI__sync_fetch_and_sub_16:
     BuiltinIndex = 1; 
     break;
       
   case Builtin::BI__sync_fetch_and_or:  
   case Builtin::BI__sync_fetch_and_or_1:
   case Builtin::BI__sync_fetch_and_or_2:
   case Builtin::BI__sync_fetch_and_or_4:
   case Builtin::BI__sync_fetch_and_or_8:
   case Builtin::BI__sync_fetch_and_or_16:
     BuiltinIndex = 2; 
     break;
       
   case Builtin::BI__sync_fetch_and_and: 
   case Builtin::BI__sync_fetch_and_and_1:
   case Builtin::BI__sync_fetch_and_and_2:

test/CodeGenOpenCL/atomic-ops.cl

+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -O0 -o - -triple=amdgcn-amd-amdhsa-opencl | FileCheck %s
+
+// Also test serialization of atomic operations here, to avoid duplicating the test.
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-pch -O0 -o %t -triple=amdgcn-amd-amdhsa-opencl
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -include-pch %t -O0 -triple=amdgcn-amd-amdhsa-opencl -emit-llvm -o - | FileCheck %s
+
+#ifndef ALREADY_INCLUDED
+#define ALREADY_INCLUDED
+
+typedef enum memory_order {
+  memory_order_relaxed = __ATOMIC_RELAXED,
+  memory_order_consume = __ATOMIC_CONSUME,
+  memory_order_acquire = __ATOMIC_ACQUIRE,
+  memory_order_release = __ATOMIC_RELEASE,
+  memory_order_acq_rel = __ATOMIC_ACQ_REL,
+  memory_order_seq_cst = __ATOMIC_SEQ_CST
+} memory_order;
+
+// ToDo: Currently LLVM only supports synchronization scope singlethread
+// and crossthread (default). Add tests for OpenCL synchronization scopes
+// after they are supported by LLVM.
+typedef enum synch_scope
+{
+  synch_scope_single_thread,
+  synch_scope_cross_thread,
+  synch_scope_ocl_work_item,
+  synch_scope_ocl_work_group,
+  synch_scope_ocl_device,
+  synch_scope_ocl_all_svm_devices,
+  synch_scope_ocl_sub_group
+} synch_scope;
+
+void fi1(atomic_int *i, int *j, int cmp) {
+  int x;
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}} seq_cst
+  x = __c11_atomic_load(i, memory_order_seq_cst);
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}} singlethread seq_cst
+  x = __c11_atomic_load(i, memory_order_seq_cst, synch_scope_single_thread);
+  // CHECK: load atomic i32, i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}} seq_cst
+  x = __c11_atomic_load(i, memory_order_seq_cst, synch_scope_cross_thread);
+  // CHECK: store atomic i32 %{{[.0-9A-Z_a-z]+}}, i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}} singlethread seq_cst
+  __c11_atomic_store(i, 1, memory_order_seq_cst, synch_scope_single_thread);
+  // CHECK: atomicrmw and i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}}, i32 %{{[.0-9A-Z_a-z]+}} singlethread seq_cst
+  x = __c11_atomic_fetch_and(i, 1, memory_order_seq_cst, synch_scope_single_thread);
+  // CHECK: cmpxchg i32 addrspace(4)* %{{[.0-9A-Z_a-z]+}}, i32 %{{[.0-9A-Z_a-z]+}}, i32 %{{[.0-9A-Z_a-z]+}} singlethread acquire acquire
+  x = __c11_atomic_compare_exchange_strong(i, &cmp, 1, memory_order_acquire, memory_order_acquire, synch_scope_single_thread);
+}
+
+#endif
 No newline at end of file

test/Sema/atomic-ops.c

 void f(_Atomic(int) *i, const _Atomic(int) *ci,
        _Atomic(int*) *p, _Atomic(float) *d,
        int *I, const int *CI,
        int **P, float *D, struct S *s1, struct S *s2) {
   __c11_atomic_init(I, 5); // expected-error {{pointer to _Atomic}}
   __c11_atomic_init(ci, 5); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const _Atomic(int) *' invalid)}}
 
   __c11_atomic_load(0); // expected-error {{too few arguments to function}}
-  __c11_atomic_load(0,0,0); // expected-error {{too many arguments to function}}
+  __c11_atomic_load(0,0,0,0); // expected-error {{too many arguments to function}}
   __c11_atomic_store(0,0,0); // expected-error {{address argument to atomic builtin must be a pointer}}
   __c11_atomic_store((int*)0,0,0); // expected-error {{address argument to atomic operation must be a pointer to _Atomic}}
   __c11_atomic_store(i, 0, memory_order_relaxed);
   __c11_atomic_store(ci, 0, memory_order_relaxed); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const _Atomic(int) *' invalid)}}
 
   __c11_atomic_load(i, memory_order_seq_cst);
   __c11_atomic_load(p, memory_order_seq_cst);
   __c11_atomic_load(d, memory_order_seq_cst);
   __c11_atomic_load(ci, memory_order_seq_cst); // expected-error {{address argument to atomic operation must be a pointer to non-const _Atomic type ('const _Atomic(int) *' invalid)}}
 
   int load_n_1 = __atomic_load_n(I, memory_order_relaxed);
   int *load_n_2 = __atomic_load_n(P, memory_order_relaxed);
   float load_n_3 = __atomic_load_n(D, memory_order_relaxed); // expected-error {{must be a pointer to integer or pointer}}
   __atomic_load_n(s1, memory_order_relaxed); // expected-error {{must be a pointer to integer or pointer}}
   load_n_1 = __atomic_load_n(CI, memory_order_relaxed);
 
   __atomic_load(i, I, memory_order_relaxed); // expected-error {{must be a pointer to a trivially-copyable type}}
   __atomic_load(CI, I, memory_order_relaxed);
 
   __atomic_load(I, i, memory_order_relaxed); // expected-warning {{passing '_Atomic(int) *' to parameter of type 'int *'}}
   __atomic_load(I, *P, memory_order_relaxed);
-  __atomic_load(I, *P, memory_order_relaxed, 42); // expected-error {{too many arguments}}
+  __atomic_load(I, *P, memory_order_relaxed, 42, 0); // expected-error {{too many arguments}}
   (int)__atomic_load(I, I, memory_order_seq_cst); // expected-error {{operand of type 'void'}}
   __atomic_load(s1, s2, memory_order_acquire);
   __atomic_load(CI, I, memory_order_relaxed);
   __atomic_load(I, CI, memory_order_relaxed); // expected-warning {{passing 'const int *' to parameter of type 'int *' discards qualifiers}}
   __atomic_load(CI, CI, memory_order_relaxed); // expected-warning {{passing 'const int *' to parameter of type 'int *' discards qualifiers}}
 
   __c11_atomic_store(i, 1, memory_order_seq_cst);
   __c11_atomic_store(p, 1, memory_order_seq_cst); // expected-warning {{incompatible integer to pointer conversion}}