[clang][sema] Generate builtin operator overloads for (volatile) _Atomic types

[jansvoboda11]

Summary

We observed a failed assert in overloaded compound-assignment operator resolution:

Assertion failed: (Result.isInvalid() && "C++ binary operator overloading is missing candidates!"), function CreateOverloadedBinOp, file SemaOverload.cpp, line 13944.
...
frame #4: clang` clang::Sema::CreateOverloadedBinOp(..., Opc=BO_OrAssign, ..., PerformADL=true, AllowRewrittenCandidates=false, ...) at SemaOverload.cpp:13943
frame #5: clang` BuildOverloadedBinOp(..., Opc=BO_OrAssign, ...) at SemaExpr.cpp:15228
frame #6: clang` clang::Sema::BuildBinOp(..., Opc=BO_OrAssign, ...) at SemaExpr.cpp:15330
frame #7: clang` clang::Sema::ActOnBinOp(..., Kind=pipeequal, ...) at SemaExpr.cpp:15187
frame #8: clang` clang::Parser::ParseRHSOfBinaryExpression(..., MinPrec=Assignment) at ParseExpr.cpp:629
frame #9: clang` clang::Parser::ParseAssignmentExpression(..., isTypeCast=NotTypeCast) at ParseExpr.cpp:176
frame #10: clang` clang::Parser::ParseExpression(... isTypeCast=NotTypeCast) at ParseExpr.cpp:124
frame #11: clang` clang::Parser::ParseExprStatement(...) at ParseStmt.cpp:464

A simple reproducer is:

_Atomic unsigned an_atomic_uint;

enum { an_enum_value = 1 };

void enum1() { an_atomic_uint += an_enum_value; }

This patch fixes the issue by generating builtin operator overloads for (volatile) _Atomic types.

[aaron.ballman]

It's interesting to note that an_atomic_uint = an_atomic_uint + an_enum_value works correctly: https://godbolt.org/z/cvP9e6nh7. I was trying to figure out whether the atomic qualifier is properly stripped for the compound operator. When I run under a debugger and dump the AST for Args[0], I get: DeclRefExpr 0x26efcf87f88 '_Atomic(unsigned int)' lvalue Var 0x26efcf44cb8 'an_atomic_uint' '_Atomic(unsigned int)' which seems like it may be the root cause of the problem here (tough to say given that this is a C extension in C++ though). The lvalue conversion that takes place for an_atomic_uint should drop the atomic qualifier per C2x 6.3.2.1p2.

clang/lib/Sema/SemaOverload.cpp

13721

The behavior isn't specific to GNU mode: https://godbolt.org/z/7dezsTEas

[ahatanak]

Is it not possible to handle this similarly to volatile unsigned? If I replace _Atomic unsigned with volatile unsigned, I see LookupOverloadedBinOp succeed without having to strip volatile because addAssignmentArithmeticOverloads adds candidates with volatile types.

[jkorous]

In D125349#3509546, @ahatanak wrote:

Is it not possible to handle this similarly to volatile unsigned? If I replace _Atomic unsigned with volatile unsigned, I see LookupOverloadedBinOp succeed without having to strip volatile because addAssignmentArithmeticOverloads adds candidates with volatile types.

Possibly? I am just generally apprehensive about changing the C++ side of things as I can't imagine all the consequences.
One difference that I see is the existence of volatile-qualified versions of these operators being prescribed by C++ standard while the operators you suggest definitely aren't:

// C++ [over.built]p18:
//
//   For every triple (L, VQ, R), where L is an arithmetic type,
//   VQ is either volatile or empty, and R is a promoted
//   arithmetic type, there exist candidate operator functions of
//   the form

https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaOverload.cpp#L8926

[jkorous]

In D125349#3509073, @aaron.ballman wrote:

It's interesting to note that an_atomic_uint = an_atomic_uint + an_enum_value works correctly: https://godbolt.org/z/cvP9e6nh7. I was trying to figure out whether the atomic qualifier is properly stripped for the compound operator. When I run under a debugger and dump the AST for Args[0], I get: DeclRefExpr 0x26efcf87f88 '_Atomic(unsigned int)' lvalue Var 0x26efcf44cb8 'an_atomic_uint' '_Atomic(unsigned int)' which seems like it may be the root cause of the problem here (tough to say given that this is a C extension in C++ though). The lvalue conversion that takes place for an_atomic_uint should drop the atomic qualifier per C2x 6.3.2.1p2.

Thank you for the suggestion! I'm looking into this.

[jansvoboda11]

Jan has asked me to take over.

[jansvoboda11]

Add operators for atomic types

[jansvoboda11]

I tried the approach suggested by @ahatanak. WDYT?

[rjmccall]

This seems like the right idea to me, yeah. It doesn't look like the patch handles volatile _Atomic correctly, though.

I know we do a lot of candidate prefiltering, and that that can be difficult because of uesr-defined conversions. How do those things interact with qualifiers? Like, I notice the existing code is adding both (T &, T) and (volatile T &, T) when the LHS is volatile; is there a reason that's necessary? Because we can't actually convert the LHS to remove those qualifiers, and adding combinatoric candidates could get very expensive if we start doing it for arbitrary extended qualifiers, which it feels like we ought to. (You could certainly have e.g. a volatile _Atomic __myaddrspace l-value.) If this is only necessary when the LHS has a user-defined conversion, then maybe we could at least not do it in the normal case just because the RHS is an overloaded type.

[jansvoboda11]

Handle volatile _Atomic as well

[jansvoboda11]

It doesn't look like the patch handles volatile _Atomic correctly, though.

That should be fixed in the new revision.

I know we do a lot of candidate prefiltering, and that that can be difficult because of uesr-defined conversions. How do those things interact with qualifiers?

I don't know.

Like, I notice the existing code is adding both (T &, T) and (volatile T &, T) when the LHS is volatile; is there a reason that's necessary?

Looking at Git history, that was always the case. I don't see any particular reason we do that eagerly.

Because we can't actually convert the LHS to remove those qualifiers, and adding combinatoric candidates could get very expensive if we start doing it for arbitrary extended qualifiers, which it feels like we ought to. (You could certainly have e.g. a volatile _Atomic __myaddrspace l-value.) If this is only necessary when the LHS has a user-defined conversion, then maybe we could at least not do it in the normal case just because the RHS is an overloaded type.

Yup, seems like we could be more lazy/minimal about that. I can look into that as a follow-up.

[aaron.ballman]

clang/lib/Sema/SemaOverload.cpp

9012

Do we need to handle atomic vector types?

[rjmccall]

clang/lib/Sema/SemaOverload.cpp

9002–9008

Can we do this in a way that's a little more general if we want to include other qualifiers in this combinatoric explosion in the future? I think you just need to make a recursive helper function that takes a set of qualifiers to break down (treating the atomic flag as a "qualifier" for this purpose) and then calls a callback for all combinations of those qualifiers. Like:

struct QualifiersAndAtomic {
  Qualifiers Quals;
  bool HasAtomic;
  QualifiersAndAtomic() : HasAtomic(false) {}
  QualifiersAndAtomic(Qualifiers quals, bool hasAtomic) : Quals(quals), HasAtomic(hasAtomic) {}

  QualifiersAndAtomic withVolatile() { return {Quals.withVolatile(), HasAtomic}; }
  QualifiersAndAtomic withAtomic() { return {Quals, true}; }
};

static void forAllQualifierCombinations(QualifiersAndAtomic available, QualifiersAndAtomic applied, llvm::function_ref<void(QualifiersAndAtomic)> callback) {
  // _Atomic
  if (available.HasAtomic) {
    available.HasAtomic = false;
    forAllQualifierCombinations(available, applied.withAtomic());
    forAllQualifierCombinations(available, applied);
    return;
  }

  // volatile
  if (available.Quals.hasVolatile()) {
    available.Quals.removeVolatile();
    assert(!applied.Quals.hasVolatile());
    forAllQualifierCombinations(available, applied.withVolatile());
    forAllQualifierCombinations(available, applied);
    return;
  }

  callback(applied);
}

The QualifiersAndAtomic structure is probably useful enough to go in Type.h.

[jansvoboda11]

Introduce QualifiersAndAtomic

[jansvoboda11]

clang/lib/Sema/SemaOverload.cpp

9012

I tried those, but it appears vector types don't accept _Atomic element types:

typedef _Atomic unsigned v_a_unsigned __attribute__((__vector_size__(16)));
// error: invalid vector element type '_Atomic(unsigned int)'

And _Atomic vector gives this error:

typedef unsigned v_unsigned __attribute__((__vector_size__(16)));
typedef _Atomic v_unsigned a_v_unsigned;

a_v_unsigned avu;

void enum5() { avu += an_enum_value; }
void enum6() { avu |= an_enum_value; }

I'm not an expert in this area, so I can't say for sure this is the correct behavior. But at least we don't crash.

[jansvoboda11]

clang/lib/Sema/SemaOverload.cpp

9012

And _Atomic vector gives this error:

error: invalid operands to binary expression

[rjmccall]

clang/include/clang/AST/Type.h

282

Could you go ahead and add this for const and restrict as well? I think that will give us the full set.

clang/lib/Sema/SemaOverload.cpp

8248

Extremely minor style nit: standard style is to declare these static rather than in an anonymous namespace. And it's probably worthwhile to say in a comment that we're currently only handling the qualifiers that are meaningful for the LHS of compound assignment overloading.

9012

The way to get an answer here, I think, is to see if it works with something like a single scalar int.

Unfortunately, it probably does need to work (maybe conditionally), because at least some of our supported vector types have an implicit "splat" promotion from scalars, so you have the enum -> int -> splat conversion path. But we don't have to do that in this patch.

[aaron.ballman]

clang/include/clang/AST/Type.h

627

The set of operations here feel a bit weird. We have withVolatile and withAtomic but not withConst or withRestrict. We have hasVolatile and hasRestrict but no hasConst or hasAtomic. And we have addConst but no other add methods for the other qualifiers.

Should these sets be rounded out to cover all the qualifiers? I know we don't need them for the functionality you're fixing up, but this seems like a pretty generic interface in a header file that's used all over the place.

clang/lib/Sema/SemaOverload.cpp

9012

But we don't have to do that in this patch.

+1, if it's involved, it can definitely be done in a follow-up.

[jansvoboda11]

Code review feedback

[jansvoboda11]

Implement another round of review feedback

[rjmccall]

Minor suggestion for a cleanup, then LGTM.

clang/lib/Sema/SemaOverload.cpp

9063

Can you add a helper function to build this qualified l-value reference type from a QualifiersAndAtomic?

[aaron.ballman]

LGTM modulo the suggestion from @rjmccall, thank you for this!

[jansvoboda11]

Extract function for building type

clang/include/clang/AST/Type.h

public:
   unsigned getAsOpaqueValue() const {
     return Mask;
   }
 
   bool hasConst() const { return Mask & Const; }
   bool hasOnlyConst() const { return Mask == Const; }
   void removeConst() { Mask &= ~Const; }
   void addConst() { Mask |= Const; }
+  Qualifiers withConst() const {
+    Qualifiers Qs = *this;
+    Qs.addConst();
+    return Qs;
+  }
 
   bool hasVolatile() const { return Mask & Volatile; }
   bool hasOnlyVolatile() const { return Mask == Volatile; }
   void removeVolatile() { Mask &= ~Volatile; }
   void addVolatile() { Mask |= Volatile; }
+  Qualifiers withVolatile() const {
+    Qualifiers Qs = *this;
+    Qs.addVolatile();
+    return Qs;
+  }
 
   bool hasRestrict() const { return Mask & Restrict; }
   bool hasOnlyRestrict() const { return Mask == Restrict; }
   void removeRestrict() { Mask &= ~Restrict; }
   void addRestrict() { Mask |= Restrict; }
+  Qualifiers withRestrict() const {
+    Qualifiers Qs = *this;
+    Qs.addRestrict();
+    return Qs;
+  }
 
   bool hasCVRQualifiers() const { return getCVRQualifiers(); }
   unsigned getCVRQualifiers() const { return Mask & CVRMask; }
   unsigned getCVRUQualifiers() const { return Mask & (CVRMask | UMask); }
 
   void setCVRQualifiers(unsigned mask) {
     assert(!(mask & ~CVRMask) && "bitmask contains non-CVR bits");
     Mask = (Mask & ~CVRMask) | mask;
private:
   static const uint32_t GCAttrShift = 4;
   static const uint32_t LifetimeMask = 0x1C0;
   static const uint32_t LifetimeShift = 6;
   static const uint32_t AddressSpaceMask =
       ~(CVRMask | UMask | GCAttrMask | LifetimeMask);
   static const uint32_t AddressSpaceShift = 9;
 };
 
+class QualifiersAndAtomic {
+  Qualifiers Quals;
+  bool HasAtomic;
+
+public:
+  QualifiersAndAtomic() : HasAtomic(false) {}
+  QualifiersAndAtomic(Qualifiers Quals, bool HasAtomic)
+      : Quals(Quals), HasAtomic(HasAtomic) {}
+
+  operator Qualifiers() const { return Quals; }
+
+  bool hasVolatile() const { return Quals.hasVolatile(); }
+  bool hasConst() const { return Quals.hasConst(); }
+  bool hasRestrict() const { return Quals.hasRestrict(); }
+  bool hasAtomic() const { return HasAtomic; }
+
+  void addVolatile() { Quals.addVolatile(); }
+  void addConst() { Quals.addConst(); }
+  void addRestrict() { Quals.addRestrict(); }
+  void addAtomic() { HasAtomic = true; }
+
+  void removeVolatile() { Quals.removeVolatile(); }
+  void removeConst() { Quals.removeConst(); }
+  void removeRestrict() { Quals.removeRestrict(); }
+  void removeAtomic() { HasAtomic = false; }
+
+  QualifiersAndAtomic withVolatile() {
+    return {Quals.withVolatile(), HasAtomic};
+  }
+  QualifiersAndAtomic withConst() { return {Quals.withConst(), HasAtomic}; }
+  QualifiersAndAtomic withRestrict() {
+    return {Quals.withRestrict(), HasAtomic};
+  }
+  QualifiersAndAtomic withAtomic() { return {Quals, true}; }
+
+  QualifiersAndAtomic &operator+=(Qualifiers RHS) {
+    Quals += RHS;
+    return *this;
+  }
+};
+
 /// A std::pair-like structure for storing a qualified type split
 /// into its local qualifiers and its locally-unqualified type.
 struct SplitQualType {
   /// The locally-unqualified type.
   const Type *Ty = nullptr;
 
   /// The local qualifiers.
   Qualifiers Quals;

clang/lib/Sema/SemaOverload.cpp

for (NamedDecl *D : ClassDecl->getVisibleConversionFunctions()) {
           if (VRQuals.hasRestrict() && VRQuals.hasVolatile())
             return VRQuals;
         }
       }
     }
     return VRQuals;
 }
 
+// Note: We're currently only handling qualifiers that are meaningful for the
+// LHS of compound assignment overloading.
+static void forAllQualifierCombinationsImpl(
+    QualifiersAndAtomic Available, QualifiersAndAtomic Applied,
+    llvm::function_ref<void(QualifiersAndAtomic)> Callback) {
+  // _Atomic
+  if (Available.hasAtomic()) {
+    Available.removeAtomic();
+    forAllQualifierCombinationsImpl(Available, Applied.withAtomic(), Callback);
+    forAllQualifierCombinationsImpl(Available, Applied, Callback);
+    return;
+  }
+
+  // volatile
+  if (Available.hasVolatile()) {
+    Available.removeVolatile();
+    assert(!Applied.hasVolatile());
+    forAllQualifierCombinationsImpl(Available, Applied.withVolatile(),
+                                    Callback);
+    forAllQualifierCombinationsImpl(Available, Applied, Callback);
+    return;
+  }
+
+  Callback(Applied);
+}
+
+static void forAllQualifierCombinations(
+    QualifiersAndAtomic Quals,
+    llvm::function_ref<void(QualifiersAndAtomic)> Callback) {
+  return forAllQualifierCombinationsImpl(Quals, QualifiersAndAtomic(),
+                                         Callback);
+}
+
+static QualType makeQualifiedLValueReferenceType(QualType Base,
+                                                 QualifiersAndAtomic Quals,
+                                                 Sema &S) {
+  if (Quals.hasAtomic())
+    Base = S.Context.getAtomicType(Base);
+  if (Quals.hasVolatile())
+    Base = S.Context.getVolatileType(Base);
+  return S.Context.getLValueReferenceType(Base);
+}
+
 namespace {
 
 /// Helper class to manage the addition of builtin operator overload
 /// candidates. It provides shared state and utility methods used throughout
 /// the process, as well as a helper method to add each group of builtin
 /// operator overloads from the standard to a candidate set.
 class BuiltinOperatorOverloadBuilder {
   // Common instance state available to all overload candidate addition methods.
   Sema &S;
   ArrayRef<Expr *> Args;
-  Qualifiers VisibleTypeConversionsQuals;
+  QualifiersAndAtomic VisibleTypeConversionsQuals;
   bool HasArithmeticOrEnumeralCandidateType;
   SmallVectorImpl<BuiltinCandidateTypeSet> &CandidateTypes;
   OverloadCandidateSet &CandidateSet;
 
   static constexpr int ArithmeticTypesCap = 24;
   SmallVector<CanQualType, ArithmeticTypesCap> ArithmeticTypes;
 
   // Define some indices used to iterate over the arithmetic types in
class BuiltinOperatorOverloadBuilder {
   void AddCandidate(QualType L, QualType R) {
     QualType LandR[2] = {L, R};
     S.AddBuiltinCandidate(LandR, Args, CandidateSet);
   }
 
 public:
   BuiltinOperatorOverloadBuilder(
     Sema &S, ArrayRef<Expr *> Args,
-    Qualifiers VisibleTypeConversionsQuals,
+    QualifiersAndAtomic VisibleTypeConversionsQuals,
     bool HasArithmeticOrEnumeralCandidateType,
     SmallVectorImpl<BuiltinCandidateTypeSet> &CandidateTypes,
     OverloadCandidateSet &CandidateSet)
     : S(S), Args(Args),
       VisibleTypeConversionsQuals(VisibleTypeConversionsQuals),
       HasArithmeticOrEnumeralCandidateType(
         HasArithmeticOrEnumeralCandidateType),
       CandidateTypes(CandidateTypes),
void addAssignmentArithmeticOverloads(bool isEqualOp) {
 
     for (unsigned Left = 0; Left < NumArithmeticTypes; ++Left) {
       for (unsigned Right = FirstPromotedArithmeticType;
            Right < LastPromotedArithmeticType; ++Right) {
         QualType ParamTypes[2];
         ParamTypes[1] = ArithmeticTypes[Right];
         auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
             S, ArithmeticTypes[Left], Args[0]);
-        // Add this built-in operator as a candidate (VQ is empty).
-        ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
-        S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
-                              /*IsAssignmentOperator=*/isEqualOp);
 
-        // Add this built-in operator as a candidate (VQ is 'volatile').
-        if (VisibleTypeConversionsQuals.hasVolatile()) {
-          ParamTypes[0] = S.Context.getVolatileType(LeftBaseTy);
-          ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
+        forAllQualifierCombinations(
+            VisibleTypeConversionsQuals, [&](QualifiersAndAtomic Quals) {
+              ParamTypes[0] =
+                  makeQualifiedLValueReferenceType(LeftBaseTy, Quals, S);
               S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
                                     /*IsAssignmentOperator=*/isEqualOp);
-        }
+            });
       }
     }
 
     // Extension: Add the binary operators =, +=, -=, *=, /= for vector types.
     for (QualType Vec1Ty : CandidateTypes[0].vector_types())
       for (QualType Vec2Ty : CandidateTypes[0].vector_types()) {
         QualType ParamTypes[2];
         ParamTypes[1] = Vec2Ty;
         // Add this built-in operator as a candidate (VQ is empty).
         ParamTypes[0] = S.Context.getLValueReferenceType(Vec1Ty);
         S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
                               /*IsAssignmentOperator=*/isEqualOp);
void addAssignmentIntegralOverloads() {
 
     for (unsigned Left = FirstIntegralType; Left < LastIntegralType; ++Left) {
       for (unsigned Right = FirstPromotedIntegralType;
            Right < LastPromotedIntegralType; ++Right) {
         QualType ParamTypes[2];
         ParamTypes[1] = ArithmeticTypes[Right];
         auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
             S, ArithmeticTypes[Left], Args[0]);
-        // Add this built-in operator as a candidate (VQ is empty).
-        ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
-        S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
-        if (VisibleTypeConversionsQuals.hasVolatile()) {
-          // Add this built-in operator as a candidate (VQ is 'volatile').
-          ParamTypes[0] = LeftBaseTy;
-          ParamTypes[0] = S.Context.getVolatileType(ParamTypes[0]);
-          ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
+
+        forAllQualifierCombinations(
+            VisibleTypeConversionsQuals, [&](QualifiersAndAtomic Quals) {
+              ParamTypes[0] =
+                  makeQualifiedLValueReferenceType(LeftBaseTy, Quals, S);
               S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
-        }
+            });
       }
     }
   }
 
   // C++ [over.operator]p23:
   //
   //   There also exist candidate operator functions of the form
   //
   //        bool        operator!(bool);
   //        bool        operator&&(bool, bool);

 void Sema::AddBuiltinOperatorCandidates(OverloadedOperatorKind Op,
                                         SourceLocation OpLoc,
                                         ArrayRef<Expr *> Args,
                                         OverloadCandidateSet &CandidateSet) {
   // Find all of the types that the arguments can convert to, but only
   // if the operator we're looking at has built-in operator candidates
   // that make use of these types. Also record whether we encounter non-record
   // candidate types or either arithmetic or enumeral candidate types.
-  Qualifiers VisibleTypeConversionsQuals;
+  QualifiersAndAtomic VisibleTypeConversionsQuals;
   VisibleTypeConversionsQuals.addConst();
-  for (unsigned ArgIdx = 0, N = Args.size(); ArgIdx != N; ++ArgIdx)
+  for (unsigned ArgIdx = 0, N = Args.size(); ArgIdx != N; ++ArgIdx) {
     VisibleTypeConversionsQuals += CollectVRQualifiers(Context, Args[ArgIdx]);
+    if (Args[ArgIdx]->getType()->isAtomicType())
+      VisibleTypeConversionsQuals.addAtomic();
+  }
 
   bool HasNonRecordCandidateType = false;
   bool HasArithmeticOrEnumeralCandidateType = false;
   SmallVector<BuiltinCandidateTypeSet, 2> CandidateTypes;
   for (unsigned ArgIdx = 0, N = Args.size(); ArgIdx != N; ++ArgIdx) {
     CandidateTypes.emplace_back(*this);
     CandidateTypes[ArgIdx].AddTypesConvertedFrom(Args[ArgIdx]->getType(),
                                                  OpLoc,
ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
   bool HadMultipleCandidates = (CandidateSet.size() > 1);
 
   // Perform overload resolution.
   OverloadCandidateSet::iterator Best;
   switch (CandidateSet.BestViableFunction(*this, OpLoc, Best)) {
     case OR_Success: {
       // We found a built-in operator or an overloaded operator.
       FunctionDecl *FnDecl = Best->Function;
 
       bool IsReversed = Best->isReversed();
       if (IsReversed)
         std::swap(Args[0], Args[1]);
 
       if (FnDecl) {
         Expr *Base = nullptr;
         // We matched an overloaded operator. Build a call to that
         // operator.

clang/test/CodeGenCXX/atomic-builtin-compound-assignment-overload.cpp

+// RUN: %clang_cc1 -std=gnu++11 -emit-llvm -triple=x86_64-linux-gnu -o - %s | FileCheck %s
+
+_Atomic unsigned an_atomic_uint;
+
+enum { an_enum_value = 1 };
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum1v()
+void enum1() {
+  an_atomic_uint += an_enum_value;
+  // CHECK: atomicrmw add ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum2v()
+void enum2() {
+  an_atomic_uint |= an_enum_value;
+  // CHECK: atomicrmw or ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum3RU7_Atomicj({{.*}})
+void enum3(_Atomic unsigned &an_atomic_uint_param) {
+  an_atomic_uint_param += an_enum_value;
+  // CHECK: atomicrmw add ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum4RU7_Atomicj({{.*}})
+void enum4(_Atomic unsigned &an_atomic_uint_param) {
+  an_atomic_uint_param |= an_enum_value;
+  // CHECK: atomicrmw or ptr
+}
+
+volatile _Atomic unsigned an_volatile_atomic_uint;
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum5v()
+void enum5() {
+  an_volatile_atomic_uint += an_enum_value;
+  // CHECK: atomicrmw add ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum6v()
+void enum6() {
+  an_volatile_atomic_uint |= an_enum_value;
+  // CHECK: atomicrmw or ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum7RVU7_Atomicj({{.*}})
+void enum7(volatile _Atomic unsigned &an_volatile_atomic_uint_param) {
+  an_volatile_atomic_uint_param += an_enum_value;
+  // CHECK: atomicrmw add ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum8RVU7_Atomicj({{.*}})
+void enum8(volatile _Atomic unsigned &an_volatile_atomic_uint_param) {
+  an_volatile_atomic_uint_param |= an_enum_value;
+  // CHECK: atomicrmw or ptr
+}

clang/test/SemaCXX/atomic-builtin-compound-assignment-overload.cpp

+// RUN: %clang_cc1 -std=gnu++98 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+_Atomic unsigned an_atomic_uint;
+
+enum { an_enum_value = 1 };
+
+void enum1() { an_atomic_uint += an_enum_value; }
+
+void enum2() { an_atomic_uint |= an_enum_value; }
+
+volatile _Atomic unsigned an_volatile_atomic_uint;
+
+void enum3() { an_volatile_atomic_uint += an_enum_value; }
+
+void enum4() { an_volatile_atomic_uint |= an_enum_value; }