================
Comment at: lib/CodeGen/CGAtomic.cpp:1011
@@ +1010,3 @@
+ AtomicInfo AI(*this, LV);
+ return CGM.getCodeGenOpts().MSVolatile && !AI.shouldUseLibcall() &&
+ (LV.isVolatile() || hasVolatileMember(LV.getType()));
----------------
I don't get the !AI.shouldUseLibcall() part.
If the atomic operation would use a libcall, then you don't turn volatile
access into atomic? Why? How is it related to libcalls?
================
Comment at: lib/CodeGen/CGAtomic.cpp:1017
@@ +1016,3 @@
+ return CGM.getCodeGenOpts().MSVolatile &&
+ getContext().getTargetInfo().hasBuiltinAtomic(
+ getContext().getTypeSize(Ty), getContext().getTypeAlign(Ty));
----------------
Same here. How presence of builtin atomic affects this?
================
Comment at: lib/CodeGen/CGAtomic.cpp:1018
@@ +1017,3 @@
+ getContext().getTargetInfo().hasBuiltinAtomic(
+ getContext().getTypeSize(Ty), getContext().getTypeAlign(Ty));
+}
----------------
In volatileIsAtomic you check that the value is volatile, but here you don't.
Please make then consistent. Either check in both or don't check in both.
================
Comment at: lib/CodeGen/CGExpr.cpp:1141
@@ -1138,3 +1140,3 @@
// Atomic operations have to be done on integral types.
- if (Ty->isAtomicType()) {
+ if (Ty->isAtomicType() || VolatileIsAtomic) {
LValue lvalue = LValue::MakeAddr(Addr, Ty,
----------------
Won't this be overly fragile?
MS semantics effectively turn volatile variables into atomic variables. So the
current isAtomicType becomes effectively useless, and whoever uses isAtomicType
must also remember to add volatileIsAtomic. And there is nothing that reminds
of that. Ty->isAtomicType() looks pretty much like what you need.
It would be less fragile if you fix the isAtomic predicate itself.
But I know very little about llvm internals, maybe it is not possible, or a bad
idea for some other reason. I don't know.
================
Comment at: lib/CodeGen/CodeGenFunction.h:2159
@@ +2158,3 @@
+ } else {
+ IsSeqCst = false;
+ IsVolatile = true;
----------------
Here you use IsSeqCst, but in EmitAtomicStore you use AO.
Please make them consistent.
http://reviews.llvm.org/D7580
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
