[PATCH] MS ABI: Implement /volatile:ms

Reid Kleckner rnk at google.com
Thu Feb 12 13:48:42 PST 2015


Looks good to me. Feel free to gather other feedback.


================
Comment at: lib/CodeGen/CGAtomic.cpp:1017
@@ +1016,3 @@
+  bool AtomicIsInline = !AI.shouldUseLibcall();
+  return CGM.getCodeGenOpts().MSVolatile && IsVolatile && AtomicIsInline;
+}
----------------
Early return false if !CGOpts.MSVolatile before checking struct types for volatile members (not O(1))?

================
Comment at: lib/CodeGen/CGAtomic.cpp:1023
@@ +1022,3 @@
+/// performing such an operation can be performed without a libcall.
+bool CodeGenFunction::TypeIsSuitableForInlineAtomic(QualType Ty,
+                                                    bool IsVolatile) const {
----------------
Make the leading letter lowercase? "LValueIsSuitable..." starts with an initialism, so it stays upper.

================
Comment at: lib/CodeGen/CodeGenFunction.h:2152-2153
@@ +2151,4 @@
+
+  RValue EmitAtomicLoad(LValue LV, SourceLocation SL,
+                        AggValueSlot Slot = AggValueSlot::ignored()) {
+    llvm::AtomicOrdering AO;
----------------
Big enough to make not inline?

================
Comment at: lib/CodeGen/CodeGenFunction.h:2171
@@ +2170,3 @@
+                       bool IsVolatile, bool isInit);
+  void EmitAtomicStore(RValue rvalue, LValue lvalue, bool isInit) {
+    bool IsVolatile = lvalue.isVolatileQualified();
----------------
ditto

http://reviews.llvm.org/D7580

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list