jansvoboda11 updated this revision to Diff 435927.
jansvoboda11 added a comment.
Code review feedback
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125349/new/
https://reviews.llvm.org/D125349
Files:
clang/include/clang/AST/Type.h
clang/lib/Sema/SemaOverload.cpp
clang/test/CodeGenCXX/atomic-builtin-compound-assignment-overload.cpp
clang/test/SemaCXX/atomic-builtin-compound-assignment-overload.cpp
Index: clang/test/SemaCXX/atomic-builtin-compound-assignment-overload.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/atomic-builtin-compound-assignment-overload.cpp
@@ -0,0 +1,16 @@
+// 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; }
Index: clang/test/CodeGenCXX/atomic-builtin-compound-assignment-overload.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/atomic-builtin-compound-assignment-overload.cpp
@@ -0,0 +1,55 @@
+// 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
+}
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -8191,6 +8191,39 @@
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.HasAtomic = false;
+ forAllQualifierCombinationsImpl(available, applied.withAtomic(), callback);
+ forAllQualifierCombinationsImpl(available, applied, callback);
+ return;
+ }
+
+ // volatile
+ if (available.Quals.hasVolatile()) {
+ available.Quals.removeVolatile();
+ assert(!applied.Quals.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);
+}
+
namespace {
/// Helper class to manage the addition of builtin operator overload
@@ -8201,7 +8234,7 @@
// 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;
@@ -8325,7 +8358,7 @@
public:
BuiltinOperatorOverloadBuilder(
Sema &S, ArrayRef<Expr *> Args,
- Qualifiers VisibleTypeConversionsQuals,
+ QualifiersAndAtomic VisibleTypeConversionsQuals,
bool HasArithmeticOrEnumeralCandidateType,
SmallVectorImpl<BuiltinCandidateTypeSet> &CandidateTypes,
OverloadCandidateSet &CandidateSet)
@@ -8946,18 +8979,18 @@
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]);
- S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
- /*IsAssignmentOperator=*/isEqualOp);
- }
+ forAllQualifierCombinations(
+ VisibleTypeConversionsQuals, [&](QualifiersAndAtomic Quals) {
+ ParamTypes[0] = LeftBaseTy;
+ if (Quals.HasAtomic)
+ ParamTypes[0] = S.Context.getAtomicType(ParamTypes[0]);
+ if (Quals.hasVolatile())
+ ParamTypes[0] = S.Context.getVolatileType(ParamTypes[0]);
+ ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
+ S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
+ /*IsAssignmentOperator=*/isEqualOp);
+ });
}
}
@@ -9004,16 +9037,17 @@
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]);
- S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
- }
+
+ forAllQualifierCombinations(
+ VisibleTypeConversionsQuals, [&](QualifiersAndAtomic Quals) {
+ ParamTypes[0] = LeftBaseTy;
+ if (Quals.HasAtomic)
+ ParamTypes[0] = S.Context.getAtomicType(ParamTypes[0]);
+ if (Quals.hasVolatile())
+ ParamTypes[0] = S.Context.getVolatileType(ParamTypes[0]);
+ ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
+ S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
+ });
}
}
}
@@ -9177,10 +9211,12 @@
// 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]);
+ VisibleTypeConversionsQuals.HasAtomic |= Args[ArgIdx]->getType()->isAtomicType();
+ }
bool HasNonRecordCandidateType = false;
bool HasArithmeticOrEnumeralCandidateType = false;
Index: clang/include/clang/AST/Type.h
===================================================================
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -265,16 +265,31 @@
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; }
@@ -609,6 +624,31 @@
static const uint32_t AddressSpaceShift = 9;
};
+struct QualifiersAndAtomic {
+ Qualifiers Quals;
+ bool HasAtomic;
+ QualifiersAndAtomic() : HasAtomic(false) {}
+ QualifiersAndAtomic(Qualifiers Quals, bool HasAtomic)
+ : Quals(Quals), HasAtomic(HasAtomic) {}
+
+ operator Qualifiers() const { return Quals; }
+
+ QualifiersAndAtomic withVolatile() {
+ return {Quals.withVolatile(), HasAtomic};
+ }
+ QualifiersAndAtomic withAtomic() { return {Quals, true}; }
+
+ void addConst() { Quals.addConst(); }
+
+ QualifiersAndAtomic &operator+=(Qualifiers RHS) {
+ Quals += RHS;
+ return *this;
+ }
+
+ bool hasVolatile() const { return Quals.hasVolatile(); }
+ bool hasRestrict() const { return Quals.hasRestrict(); }
+};
+
/// A std::pair-like structure for storing a qualified type split
/// into its local qualifiers and its locally-unqualified type.
struct SplitQualType {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
