[clang] b02d970 - [clang][sema] Generate builtin operator overloads for (volatile) _Atomic types
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 20 02:03:36 PDT 2022
Author: Jan Svoboda
Date: 2022-06-20T11:03:29+02:00
New Revision: b02d970b4335561940dd584f6e6497ab684c788d
URL: https://github.com/llvm/llvm-project/commit/b02d970b4335561940dd584f6e6497ab684c788d
DIFF: https://github.com/llvm/llvm-project/commit/b02d970b4335561940dd584f6e6497ab684c788d.diff
LOG: [clang][sema] Generate builtin operator overloads for (volatile) _Atomic types
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.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D125349
Added:
clang/test/CodeGenCXX/atomic-builtin-compound-assignment-overload.cpp
clang/test/SemaCXX/atomic-builtin-compound-assignment-overload.cpp
Modified:
clang/include/clang/AST/Type.h
clang/lib/Sema/SemaOverload.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 5784839f4f961..1f8086301f66e 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -265,16 +265,31 @@ class Qualifiers {
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,47 @@ class Qualifiers {
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 {
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index a87c31d6a213d..d1bc5373c5339 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -8200,6 +8200,49 @@ static Qualifiers CollectVRQualifiers(ASTContext &Context, Expr* ArgExpr) {
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
@@ -8210,7 +8253,7 @@ 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;
@@ -8334,7 +8377,7 @@ class BuiltinOperatorOverloadBuilder {
public:
BuiltinOperatorOverloadBuilder(
Sema &S, ArrayRef<Expr *> Args,
- Qualifiers VisibleTypeConversionsQuals,
+ QualifiersAndAtomic VisibleTypeConversionsQuals,
bool HasArithmeticOrEnumeralCandidateType,
SmallVectorImpl<BuiltinCandidateTypeSet> &CandidateTypes,
OverloadCandidateSet &CandidateSet)
@@ -8955,18 +8998,14 @@ class BuiltinOperatorOverloadBuilder {
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] =
+ makeQualifiedLValueReferenceType(LeftBaseTy, Quals, S);
+ S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
+ /*IsAssignmentOperator=*/isEqualOp);
+ });
}
}
@@ -9013,16 +9052,13 @@ class BuiltinOperatorOverloadBuilder {
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] =
+ makeQualifiedLValueReferenceType(LeftBaseTy, Quals, S);
+ S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
+ });
}
}
}
@@ -9186,10 +9222,13 @@ void Sema::AddBuiltinOperatorCandidates(OverloadedOperatorKind Op,
// 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;
diff --git a/clang/test/CodeGenCXX/atomic-builtin-compound-assignment-overload.cpp b/clang/test/CodeGenCXX/atomic-builtin-compound-assignment-overload.cpp
new file mode 100644
index 0000000000000..fe448383107dd
--- /dev/null
+++ b/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
+}
diff --git a/clang/test/SemaCXX/atomic-builtin-compound-assignment-overload.cpp b/clang/test/SemaCXX/atomic-builtin-compound-assignment-overload.cpp
new file mode 100644
index 0000000000000..b2bc5f84b2779
--- /dev/null
+++ b/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; }
More information about the cfe-commits
mailing list