Skip to content
This repository was archived by the owner on Nov 1, 2021. It is now read-only.

Commit a9bdd3b

Browse files
committed
MS ABI: Implement /volatile:ms
The /volatile:ms semantics turn volatile loads and stores into atomic acquire and release operations. This distinction is important because volatile memory operations do not form a happens-before relationship with non-atomic memory. This means that a volatile store is not sufficient for implementing a mutex unlock routine. Differential Revision: http://reviews.llvm.org/D7580 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@229082 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent f8bd6d3 commit a9bdd3b

14 files changed

+181
-26
lines changed

Diff for: include/clang/Driver/CLCompatOptions.td

+5-3
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ def _SLASH_arch : CLCompileJoined<"arch:">,
157157
HelpText<"Set architecture for code generation">;
158158

159159
def _SLASH_M_Group : OptionGroup<"</M group>">, Group<cl_compile_Group>;
160+
def _SLASH_volatile_Group : OptionGroup<"</volatile group>">, Group<cl_compile_Group>;
160161

161162
def _SLASH_EH : CLJoined<"EH">, HelpText<"Exception handling model">;
162163
def _SLASH_EP : CLFlag<"EP">,
@@ -201,7 +202,10 @@ def _SLASH_TC : CLCompileFlag<"TC">, HelpText<"Treat all source files as C">;
201202
def _SLASH_Tp : CLCompileJoinedOrSeparate<"Tp">,
202203
HelpText<"Specify a C++ source file">, MetaVarName<"<filename>">;
203204
def _SLASH_TP : CLCompileFlag<"TP">, HelpText<"Treat all source files as C++">;
204-
205+
def _SLASH_volatile_iso : Option<["/", "-"], "volatile:iso", KIND_FLAG>, Group<_SLASH_volatile_Group>,
206+
Flags<[CLOption, DriverOption]>, HelpText<"Volatile loads and stores have standard semantics">;
207+
def _SLASH_volatile_ms : Option<["/", "-"], "volatile:ms", KIND_FLAG>, Group<_SLASH_volatile_Group>,
208+
Flags<[CLOption, DriverOption]>, HelpText<"Volatile loads and stores have acquire and release semantics">;
205209

206210
// Ignored:
207211

@@ -220,7 +224,6 @@ def _SLASH_Ob2 : CLIgnoredFlag<"Ob2">;
220224
def _SLASH_RTC : CLIgnoredJoined<"RTC">;
221225
def _SLASH_sdl : CLIgnoredFlag<"sdl">;
222226
def _SLASH_sdl_ : CLIgnoredFlag<"sdl-">;
223-
def _SLASH_volatile_iso : CLIgnoredFlag<"volatile:iso">;
224227
def _SLASH_w : CLIgnoredJoined<"w">;
225228
def _SLASH_Zc_auto : CLIgnoredFlag<"Zc:auto">;
226229
def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;
@@ -281,7 +284,6 @@ def _SLASH_Qpar : CLFlag<"Qpar">;
281284
def _SLASH_Qvec_report : CLJoined<"Qvec-report">;
282285
def _SLASH_u : CLFlag<"u">;
283286
def _SLASH_V : CLFlag<"V">;
284-
def _SLASH_volatile_ms : CLFlag<"volatile:ms">;
285287
def _SLASH_WL : CLFlag<"WL">;
286288
def _SLASH_Wp64 : CLFlag<"Wp64">;
287289
def _SLASH_X : CLFlag<"X">;

Diff for: include/clang/Driver/Options.td

+1
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,7 @@ def fms_extensions : Flag<["-"], "fms-extensions">, Group<f_Group>, Flags<[CC1Op
628628
HelpText<"Accept some non-standard constructs supported by the Microsoft compiler">;
629629
def fms_compatibility : Flag<["-"], "fms-compatibility">, Group<f_Group>, Flags<[CC1Option]>,
630630
HelpText<"Enable full Microsoft Visual C++ compatibility">;
631+
def fms_volatile : Joined<["-"], "fms-volatile">, Group<f_Group>, Flags<[CC1Option]>;
631632
def fmsc_version : Joined<["-"], "fmsc-version=">, Group<f_Group>, Flags<[DriverOption, CoreOption]>,
632633
HelpText<"Microsoft compiler version number to report in _MSC_VER (0 = don't define it (default))">;
633634
def fms_compatibility_version

Diff for: include/clang/Frontend/CodeGenOptions.def

+1
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ CODEGENOPT(LessPreciseFPMAD , 1, 0) ///< Enable less precise MAD instructions t
6868
///< be generated.
6969
CODEGENOPT(MergeAllConstants , 1, 1) ///< Merge identical constants.
7070
CODEGENOPT(MergeFunctions , 1, 0) ///< Set when -fmerge-functions is enabled.
71+
CODEGENOPT(MSVolatile , 1, 0) ///< Set when /volatile:ms is enabled.
7172
CODEGENOPT(NoCommon , 1, 0) ///< Set when -fno-common or C++ is enabled.
7273
CODEGENOPT(NoDwarfDirectoryAsm , 1, 0) ///< Set when -fno-dwarf-directory-asm is
7374
///< enabled.

Diff for: lib/CodeGen/CGAtomic.cpp

+56-5
Original file line numberDiff line numberDiff line change
@@ -1006,9 +1006,45 @@ RValue AtomicInfo::convertIntToValue(llvm::Value *IntVal,
10061006
return convertTempToRValue(Temp, ResultSlot, Loc);
10071007
}
10081008

1009+
/// An LValue is a candidate for having its loads and stores be made atomic if
1010+
/// we are operating under /volatile:ms *and* the LValue itself is volatile and
1011+
/// performing such an operation can be performed without a libcall.
1012+
bool CodeGenFunction::LValueIsSuitableForInlineAtomic(LValue LV) {
1013+
AtomicInfo AI(*this, LV);
1014+
bool IsVolatile = LV.isVolatile() || hasVolatileMember(LV.getType());
1015+
// An atomic is inline if we don't need to use a libcall.
1016+
bool AtomicIsInline = !AI.shouldUseLibcall();
1017+
return CGM.getCodeGenOpts().MSVolatile && IsVolatile && AtomicIsInline;
1018+
}
1019+
1020+
/// An type is a candidate for having its loads and stores be made atomic if
1021+
/// we are operating under /volatile:ms *and* we know the access is volatile and
1022+
/// performing such an operation can be performed without a libcall.
1023+
bool CodeGenFunction::typeIsSuitableForInlineAtomic(QualType Ty,
1024+
bool IsVolatile) const {
1025+
// An atomic is inline if we don't need to use a libcall (e.g. it is builtin).
1026+
bool AtomicIsInline = getContext().getTargetInfo().hasBuiltinAtomic(
1027+
getContext().getTypeSize(Ty), getContext().getTypeAlign(Ty));
1028+
return CGM.getCodeGenOpts().MSVolatile && IsVolatile && AtomicIsInline;
1029+
}
1030+
1031+
RValue CodeGenFunction::EmitAtomicLoad(LValue LV, SourceLocation SL,
1032+
AggValueSlot Slot) {
1033+
llvm::AtomicOrdering AO;
1034+
bool IsVolatile = LV.isVolatileQualified();
1035+
if (LV.getType()->isAtomicType()) {
1036+
AO = llvm::SequentiallyConsistent;
1037+
} else {
1038+
AO = llvm::Acquire;
1039+
IsVolatile = true;
1040+
}
1041+
return EmitAtomicLoad(LV, SL, AO, IsVolatile, Slot);
1042+
}
1043+
10091044
/// Emit a load from an l-value of atomic type. Note that the r-value
10101045
/// we produce is an r-value of the atomic *value* type.
10111046
RValue CodeGenFunction::EmitAtomicLoad(LValue src, SourceLocation loc,
1047+
llvm::AtomicOrdering AO, bool IsVolatile,
10121048
AggValueSlot resultSlot) {
10131049
AtomicInfo atomics(*this, src);
10141050
LValue LVal = atomics.getAtomicLValue();
@@ -1060,11 +1096,11 @@ RValue CodeGenFunction::EmitAtomicLoad(LValue src, SourceLocation loc,
10601096
// Okay, we're doing this natively.
10611097
llvm::Value *addr = atomics.emitCastToAtomicIntPointer(SrcAddr);
10621098
llvm::LoadInst *load = Builder.CreateLoad(addr, "atomic-load");
1063-
load->setAtomic(llvm::SequentiallyConsistent);
1099+
load->setAtomic(AO);
10641100

10651101
// Other decoration.
10661102
load->setAlignment(src.getAlignment().getQuantity());
1067-
if (src.isVolatileQualified())
1103+
if (IsVolatile)
10681104
load->setVolatile(true);
10691105
if (src.getTBAAInfo())
10701106
CGM.DecorateInstruction(load, src.getTBAAInfo());
@@ -1161,12 +1197,27 @@ llvm::Value *AtomicInfo::convertRValueToInt(RValue RVal) const {
11611197
getAtomicAlignment().getQuantity());
11621198
}
11631199

1200+
void CodeGenFunction::EmitAtomicStore(RValue rvalue, LValue lvalue,
1201+
bool isInit) {
1202+
bool IsVolatile = lvalue.isVolatileQualified();
1203+
llvm::AtomicOrdering AO;
1204+
if (lvalue.getType()->isAtomicType()) {
1205+
AO = llvm::SequentiallyConsistent;
1206+
} else {
1207+
AO = llvm::Release;
1208+
IsVolatile = true;
1209+
}
1210+
return EmitAtomicStore(rvalue, lvalue, AO, IsVolatile, isInit);
1211+
}
1212+
11641213
/// Emit a store to an l-value of atomic type.
11651214
///
11661215
/// Note that the r-value is expected to be an r-value *of the atomic
11671216
/// type*; this means that for aggregate r-values, it should include
11681217
/// storage for any padding that was necessary.
1169-
void CodeGenFunction::EmitAtomicStore(RValue rvalue, LValue dest, bool isInit) {
1218+
void CodeGenFunction::EmitAtomicStore(RValue rvalue, LValue dest,
1219+
llvm::AtomicOrdering AO, bool IsVolatile,
1220+
bool isInit) {
11701221
// If this is an aggregate r-value, it should agree in type except
11711222
// maybe for address-space qualification.
11721223
assert(!rvalue.isAggregate() ||
@@ -1209,11 +1260,11 @@ void CodeGenFunction::EmitAtomicStore(RValue rvalue, LValue dest, bool isInit) {
12091260
llvm::StoreInst *store = Builder.CreateStore(intValue, addr);
12101261

12111262
// Initializations don't need to be atomic.
1212-
if (!isInit) store->setAtomic(llvm::SequentiallyConsistent);
1263+
if (!isInit) store->setAtomic(AO);
12131264

12141265
// Other decoration.
12151266
store->setAlignment(dest.getAlignment().getQuantity());
1216-
if (dest.isVolatileQualified())
1267+
if (IsVolatile)
12171268
store->setVolatile(true);
12181269
if (dest.getTBAAInfo())
12191270
CGM.DecorateInstruction(store, dest.getTBAAInfo());

Diff for: lib/CodeGen/CGExpr.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -1136,7 +1136,7 @@ llvm::Value *CodeGenFunction::EmitLoadOfScalar(llvm::Value *Addr, bool Volatile,
11361136
}
11371137

11381138
// Atomic operations have to be done on integral types.
1139-
if (Ty->isAtomicType()) {
1139+
if (Ty->isAtomicType() || typeIsSuitableForInlineAtomic(Ty, Volatile)) {
11401140
LValue lvalue = LValue::MakeAddr(Addr, Ty,
11411141
CharUnits::fromQuantity(Alignment),
11421142
getContext(), TBAAInfo);
@@ -1255,7 +1255,8 @@ void CodeGenFunction::EmitStoreOfScalar(llvm::Value *Value, llvm::Value *Addr,
12551255

12561256
Value = EmitToMemory(Value, Ty);
12571257

1258-
if (Ty->isAtomicType()) {
1258+
if (Ty->isAtomicType() ||
1259+
(!isInit && typeIsSuitableForInlineAtomic(Ty, Volatile))) {
12591260
EmitAtomicStore(RValue::get(Value),
12601261
LValue::MakeAddr(Addr, Ty,
12611262
CharUnits::fromQuantity(Alignment),

Diff for: lib/CodeGen/CGExprAgg.cpp

+5-3
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ void AggExprEmitter::EmitAggLoadOfLValue(const Expr *E) {
212212
LValue LV = CGF.EmitLValue(E);
213213

214214
// If the type of the l-value is atomic, then do an atomic load.
215-
if (LV.getType()->isAtomicType()) {
215+
if (LV.getType()->isAtomicType() || CGF.LValueIsSuitableForInlineAtomic(LV)) {
216216
CGF.EmitAtomicLoad(LV, E->getExprLoc(), Dest);
217217
return;
218218
}
@@ -865,7 +865,8 @@ void AggExprEmitter::VisitBinAssign(const BinaryOperator *E) {
865865
LValue LHS = CGF.EmitCheckedLValue(E->getLHS(), CodeGenFunction::TCK_Store);
866866

867867
// That copy is an atomic copy if the LHS is atomic.
868-
if (LHS.getType()->isAtomicType()) {
868+
if (LHS.getType()->isAtomicType() ||
869+
CGF.LValueIsSuitableForInlineAtomic(LHS)) {
869870
CGF.EmitAtomicStore(Dest.asRValue(), LHS, /*isInit*/ false);
870871
return;
871872
}
@@ -882,7 +883,8 @@ void AggExprEmitter::VisitBinAssign(const BinaryOperator *E) {
882883

883884
// If we have an atomic type, evaluate into the destination and then
884885
// do an atomic copy.
885-
if (LHS.getType()->isAtomicType()) {
886+
if (LHS.getType()->isAtomicType() ||
887+
CGF.LValueIsSuitableForInlineAtomic(LHS)) {
886888
EnsureDest(E->getRHS()->getType());
887889
Visit(E->getRHS());
888890
CGF.EmitAtomicStore(Dest.asRValue(), LHS, /*isInit*/ false);

Diff for: lib/CodeGen/CGExprComplex.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,8 @@ ComplexPairTy ComplexExprEmitter::EmitLoadOfLValue(LValue lvalue,
336336
/// specified value pointer.
337337
void ComplexExprEmitter::EmitStoreOfComplex(ComplexPairTy Val, LValue lvalue,
338338
bool isInit) {
339-
if (lvalue.getType()->isAtomicType())
339+
if (lvalue.getType()->isAtomicType() ||
340+
(!isInit && CGF.LValueIsSuitableForInlineAtomic(lvalue)))
340341
return CGF.EmitAtomicStore(RValue::getComplex(Val), lvalue, isInit);
341342

342343
llvm::Value *Ptr = lvalue.getAddress();

Diff for: lib/CodeGen/CGStmtOpenMP.cpp

+5-2
Original file line numberDiff line numberDiff line change
@@ -829,8 +829,11 @@ static void EmitOMPAtomicReadExpr(CodeGenFunction &CGF, bool IsSeqCst,
829829
assert(X->isLValue() && "X of 'omp atomic read' is not lvalue");
830830
LValue XLValue = CGF.EmitLValue(X);
831831
LValue VLValue = CGF.EmitLValue(V);
832-
RValue Res = XLValue.isGlobalReg() ? CGF.EmitLoadOfLValue(XLValue, Loc)
833-
: CGF.EmitAtomicLoad(XLValue, Loc);
832+
RValue Res = XLValue.isGlobalReg()
833+
? CGF.EmitLoadOfLValue(XLValue, Loc)
834+
: CGF.EmitAtomicLoad(XLValue, Loc,
835+
IsSeqCst ? llvm::SequentiallyConsistent
836+
: llvm::Monotonic);
834837
// OpenMP, 2.12.6, atomic Construct
835838
// Any atomic construct with a seq_cst clause forces the atomically
836839
// performed operation to include an implicit flush operation without a

Diff for: lib/CodeGen/CodeGenFunction.h

+10
Original file line numberDiff line numberDiff line change
@@ -2147,11 +2147,21 @@ class CodeGenFunction : public CodeGenTypeCache {
21472147

21482148
void EmitAtomicInit(Expr *E, LValue lvalue);
21492149

2150+
bool LValueIsSuitableForInlineAtomic(LValue Src);
2151+
bool typeIsSuitableForInlineAtomic(QualType Ty, bool IsVolatile) const;
2152+
2153+
RValue EmitAtomicLoad(LValue LV, SourceLocation SL,
2154+
AggValueSlot Slot = AggValueSlot::ignored());
2155+
21502156
RValue EmitAtomicLoad(LValue lvalue, SourceLocation loc,
2157+
llvm::AtomicOrdering AO, bool IsVolatile = false,
21512158
AggValueSlot slot = AggValueSlot::ignored());
21522159

21532160
void EmitAtomicStore(RValue rvalue, LValue lvalue, bool isInit);
21542161

2162+
void EmitAtomicStore(RValue rvalue, LValue lvalue, llvm::AtomicOrdering AO,
2163+
bool IsVolatile, bool isInit);
2164+
21552165
std::pair<RValue, RValue> EmitAtomicCompareExchange(
21562166
LValue Obj, RValue Expected, RValue Desired, SourceLocation Loc,
21572167
llvm::AtomicOrdering Success = llvm::SequentiallyConsistent,

Diff for: lib/Driver/Tools.cpp

+13
Original file line numberDiff line numberDiff line change
@@ -4888,6 +4888,19 @@ void Clang::AddClangCLArgs(const ArgList &Args, ArgStringList &CmdArgs) const {
48884888
CmdArgs.push_back("-P");
48894889
}
48904890

4891+
unsigned VolatileOptionID;
4892+
if (getToolChain().getTriple().getArch() == llvm::Triple::x86_64 ||
4893+
getToolChain().getTriple().getArch() == llvm::Triple::x86)
4894+
VolatileOptionID = options::OPT__SLASH_volatile_ms;
4895+
else
4896+
VolatileOptionID = options::OPT__SLASH_volatile_iso;
4897+
4898+
if (Arg *A = Args.getLastArg(options::OPT__SLASH_volatile_Group))
4899+
VolatileOptionID = A->getOption().getID();
4900+
4901+
if (VolatileOptionID == options::OPT__SLASH_volatile_ms)
4902+
CmdArgs.push_back("-fms-volatile");
4903+
48914904
Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
48924905
Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
48934906
if (MostGeneralArg && BestCaseArg)

Diff for: lib/Frontend/CompilerInvocation.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,8 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
477477
OPT_fno_data_sections, false);
478478
Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
479479

480+
Opts.MSVolatile = Args.hasArg(OPT_fms_volatile);
481+
480482
Opts.VectorizeBB = Args.hasArg(OPT_vectorize_slp_aggressive);
481483
Opts.VectorizeLoop = Args.hasArg(OPT_vectorize_loops);
482484
Opts.VectorizeSLP = Args.hasArg(OPT_vectorize_slp);

Diff for: test/CodeGen/ms-volatile.c

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// RUN: %clang_cc1 -triple i386-pc-win32 -emit-llvm -fms-volatile -o - < %s | FileCheck %s
2+
struct foo {
3+
volatile int x;
4+
};
5+
struct bar {
6+
int x;
7+
};
8+
typedef _Complex float __declspec(align(8)) baz;
9+
10+
void test1(struct foo *p, struct foo *q) {
11+
*p = *q;
12+
// CHECK-LABEL: @test1
13+
// CHECK: load atomic volatile {{.*}} acquire
14+
// CHECK: store atomic volatile {{.*}}, {{.*}} release
15+
}
16+
void test2(volatile int *p, volatile int *q) {
17+
*p = *q;
18+
// CHECK-LABEL: @test2
19+
// CHECK: load atomic volatile {{.*}} acquire
20+
// CHECK: store atomic volatile {{.*}}, {{.*}} release
21+
}
22+
void test3(struct foo *p, struct foo *q) {
23+
p->x = q->x;
24+
// CHECK-LABEL: @test3
25+
// CHECK: load atomic volatile {{.*}} acquire
26+
// CHECK: store atomic volatile {{.*}}, {{.*}} release
27+
}
28+
void test4(volatile struct foo *p, volatile struct foo *q) {
29+
p->x = q->x;
30+
// CHECK-LABEL: @test4
31+
// CHECK: load atomic volatile {{.*}} acquire
32+
// CHECK: store atomic volatile {{.*}}, {{.*}} release
33+
}
34+
void test5(volatile struct foo *p, volatile struct foo *q) {
35+
*p = *q;
36+
// CHECK-LABEL: @test5
37+
// CHECK: load atomic volatile {{.*}} acquire
38+
// CHECK: store atomic volatile {{.*}}, {{.*}} release
39+
}
40+
void test6(struct bar *p, struct bar *q) {
41+
*p = *q;
42+
// CHECK-LABEL: @test6
43+
// CHECK-NOT: load atomic volatile {{.*}}
44+
// CHECK-NOT: store atomic volatile {{.*}}, {{.*}}
45+
}
46+
void test7(volatile struct bar *p, volatile struct bar *q) {
47+
*p = *q;
48+
// CHECK-LABEL: @test7
49+
// CHECK: load atomic volatile {{.*}} acquire
50+
// CHECK: store atomic volatile {{.*}}, {{.*}} release
51+
}
52+
void test8(volatile double *p, volatile double *q) {
53+
*p = *q;
54+
// CHECK-LABEL: @test8
55+
// CHECK: load atomic volatile {{.*}} acquire
56+
// CHECK: store atomic volatile {{.*}}, {{.*}} release
57+
}
58+
void test9(volatile baz *p, baz *q) {
59+
*p = *q;
60+
// CHECK-LABEL: @test9
61+
// CHECK: store atomic volatile {{.*}}, {{.*}} release
62+
}

Diff for: test/Driver/cl-options.c

+6
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,12 @@
123123
// RUN: %clang_cl /vmg /vmm /vms -### -- %s 2>&1 | FileCheck -check-prefix=VMX %s
124124
// VMX: '/vms' not allowed with '/vmm'
125125

126+
// RUN: %clang_cl /volatile:iso -### -- %s 2>&1 | FileCheck -check-prefix=VOLATILE-ISO %s
127+
// VOLATILE-ISO-NOT: "-fms-volatile"
128+
129+
// RUN: %clang_cl /volatile:ms -### -- %s 2>&1 | FileCheck -check-prefix=VOLATILE-MS %s
130+
// VOLATILE-MS: "-fms-volatile"
131+
126132
// RUN: %clang_cl /W0 -### -- %s 2>&1 | FileCheck -check-prefix=W0 %s
127133
// W0: -w
128134

0 commit comments

Comments
 (0)