dim added inline comments.

================
Comment at: lib/Basic/Targets.cpp:4244
+    } else // allow locked atomics up to 4 bytes
+      MaxAtomicPromoteWidth = 32;
+  }
----------------
Are you purposefully not setting `MaxAtomicInlineWidth` here?  (It seems from 
`TargetInfo` that the default value is zero.)



================
Comment at: lib/Basic/Targets.cpp:4265
 
-    // x86-32 has atomics up to 8 bytes
-    // FIXME: Check that we actually have cmpxchg8b before setting
-    // MaxAtomicInlineWidth. (cmpxchg8b is an i586 instruction.)
-    MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+    setAtomic();
   }
----------------
As far as I can see, in the constructor this call is _always_ made with `CPU` 
set to `CK_Generic`, i.e. zero.  Therefore, the "allow locked atomics up to 4 
bytes" path in `setAtomic` is always chosen.  Maybe it is clearer to just 
initialize `MaxAtomicPromoteWidth` to 32 directly here, instead?



Repository:
  rL LLVM

https://reviews.llvm.org/D29542



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to