[PATCH] D42889: [LoopIdiomRecognize] Add support for memmove. Fixes PR25165

Eric Fiselier via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 3 19:44:57 PST 2018


EricWF marked 2 inline comments as done.
EricWF added inline comments.


================
Comment at: lib/IR/IRBuilder.cpp:169-186
+  if (DstAlign > 0)
+    MCI->setDestAlignment(DstAlign);
+  if (SrcAlign > 0)
+    MCI->setSourceAlignment(SrcAlign);
+
+  // Set the TBAA info if present.
+  if (TBAATag)
----------------
chandlerc wrote:
> Maybe factor this out and share it between MemCpy and MemMove?
Actually, I made a dumb mistake, and missed the already existing `CreateMemMove` function. I'll be removing this entirely. 

Perhaps it still makes sense to factor out the shared code, but that should be a separate commit.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:484
+    if (UnorderedAtomic || LI->isAtomic())
+      return HasMemcpy ? LegalStoreKind::UnorderedAtomicMemcpy
+                       : LegalStoreKind::None;
----------------
chandlerc wrote:
> Why the check for HasMemcpy here? Seems a bit odd to check it here rather than later....
We checked `HasMemcpy || HasMemmove` at the top of this block, so it's possible (but unlikely) that we only have `memmove`. Since we don't have any way to lower the atomic store, it seems logical to report `LegalStoreKind::None` right away.




https://reviews.llvm.org/D42889





More information about the llvm-commits mailing list