[PATCH] D33243: [Atomics][LoopIdiom] Recognize unordered atomic memcpy

[Daniel Neilson via Phabricator via llvm-commits]

dneilson marked 3 inline comments as done.
dneilson added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:995
+    // be a limit to min(platform register size, 16) ?
+    if (StoreSize > 16)
+      return false;
----------------
anna wrote:
> We can do 2 things here: Add a hidden `cl` option for the size that is default to 16 for now, instead of the hardcoded value. 
> 
> You could take a look if `TTI` has that target specific information you need. Perhaps `getRegisterBitWidth`?
I've changed this to query the runtime lib for whether an unordered atomic memcpy with the given store size exists. If it does, then we can create the memcpy; else we can't because we won't be able to lower it into a lib call.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1004
+      // Don't set alignment of 0
+      if (!alignment)
+        return;
----------------
anna wrote:
> If the spec requires atomic loads and stores to have an alignment, shouldn't this be an assert, rather than a check and return?
Spec requires atomic loads/stores to have alignment, but has no such requirement for non-atomic loads/stores. It's possible that one of source or dest will be non-atomic.  I'll clarify the comment.


https://reviews.llvm.org/D33243