[PATCH] D34566: [loop idiom Recognition] support memcpy for multiple consecutive loads and stores

[Haicheng Wu via Phabricator via llvm-commits]

haicheng added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:143
+                               bool NegStride, LegalStoreKind MemIdiom,
+                               bool IsLoopMemset = false);
+
----------------
DIVYA wrote:
> haicheng wrote:
> > I think we should use MemIdiom and IsLoopMemset can be removed.
> But when processLoopStridedStore() is called from processLoopMemSet() function , then IsLoopMemset is set true and when it is called from processLoopStores() function , then  IsLoopMemset is set false.Even though both can have MemIdiom set to LegalStoreKind::Memset .So using MemIdiom alone would not be sufficient.As, we need IsLoopMemset to indicate if memset which can be promoted to a large memset.
Yes, you are right.  I think we can move the check avoidLIRForMultiBlockLoop() much earlier so that we don't need this flag and we can save compilation time.  But, we can do that in the next patch.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:965-968
 
-  // The store must be feeding a non-volatile load.
-  LoadInst *LI = cast<LoadInst>(SI->getValueOperand());
-  assert(LI->isUnordered() && "Expected only non-volatile non-ordered loads.");
-
-  // See if the pointer expression is an AddRec like {base,+,1} on the current
-  // loop, which indicates a strided load.  If we have something else, it's a
-  // random load we can't handle.
-  const SCEVAddRecExpr *LoadEv =
-      cast<SCEVAddRecExpr>(SE->getSCEV(LI->getPointerOperand()));
-
-  // The trip count of the loop and the base pointer of the addrec SCEV is
-  // guaranteed to be loop invariant, which means that it should dominate the
-  // header.  This allows us to insert code for it in the preheader.
-  BasicBlock *Preheader = CurLoop->getLoopPreheader();
-  IRBuilder<> Builder(Preheader->getTerminator());
-  SCEVExpander Expander(*SE, *DL, "loop-idiom");
-
-  const SCEV *StrStart = StoreEv->getStart();
-  unsigned StrAS = SI->getPointerAddressSpace();
-  Type *IntPtrTy = Builder.getIntPtrTy(*DL, StrAS);
-
-  // Handle negative strided loops.
-  if (NegStride)
-    StrStart = getStartForNegStride(StrStart, BECount, IntPtrTy, StoreSize, SE);
-
-  // Okay, we have a strided store "p[i]" of a loaded value.  We can turn
-  // this into a memcpy in the loop preheader now if we want.  However, this
-  // would be unsafe to do if there is anything else in the loop that may read
-  // or write the memory region we're storing to.  This includes the load that
-  // feeds the stores.  Check for an alias by generating the base address and
-  // checking everything.
-  Value *StoreBasePtr = Expander.expandCodeFor(
-      StrStart, Builder.getInt8PtrTy(StrAS), Preheader->getTerminator());
-
-  SmallPtrSet<Instruction *, 1> Stores;
-  Stores.insert(SI);
-  if (mayLoopAccessLocation(StoreBasePtr, MRI_ModRef, CurLoop, BECount,
-                            StoreSize, *AA, Stores)) {
-    Expander.clear();
-    // If we generated new code for the base pointer, clean up.
-    RecursivelyDeleteTriviallyDeadInstructions(StoreBasePtr, TLI);
-    return false;
+  if (MemIdiom == LegalStoreKind::Memset ||
+      MemIdiom == LegalStoreKind::MemsetPattern) {
+    if (MemIdiom == LegalStoreKind::Memset) {
----------------
I think here can be written as

if (Memset)
else if (MemsetPattern)
else (Memcpy)

not nested if


https://reviews.llvm.org/D34566