Check dominance tree before lifting memcpy in MemCpyOpt

[deadalnix]

Summary

The load to store forwarding of aggregate to memcpy transformation can create faulty IR. This happens when the memecpy would be materialized before the argument of the store.

This diff add a check so that dominance tree remains correct.

[Prazek]

Do you have any other test case? I don't see that change of the label in the test will pick this case up.

[deadalnix]

I don't think this pays for itself. I'll come up with something buffed up here.

lib/Transforms/Scalar/MemCpyOptimizer.cpp

if (LI->isSimple() && LI->hasOneUse() &&
           if (!(AA.getModRefInfo(&*I, LoadLoc) & MRI_Mod))
             continue;
 
           // We found an instruction that may write to the loaded memory.
           // We can try to promote at this position instead of the store
           // position if nothing alias the store memory after this and the store
           // destination is not in the range.
           P = &*I;
+
+          // Make sure this position dominate store's address.
+          if (auto *Addr = dyn_cast<Instruction>(SI->getPointerOperand())) {
+            DominatorTree &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
+            if (DT.dominates(P, Addr)) {
+              P = nullptr;
+              break;
+            }
+          }
+
           for (; I != E; ++I) {
             MemoryLocation StoreLoc = MemoryLocation::get(SI);
-            if (&*I == SI->getOperand(1) ||
-                AA.getModRefInfo(&*I, StoreLoc) != MRI_NoModRef) {
+            if (AA.getModRefInfo(&*I, StoreLoc) != MRI_NoModRef) {
               P = nullptr;
               break;
             }
           }
 
           break;
         }
 

test/Transforms/MemCpyOpt/fca2memcpy.ll

 ; CHECK-NEXT: ret void
   %1 = load %S, %S* %src
   %2 = load %S, %S* %src
   store %S %1, %S* %dst
   store %S %2, %S* %dst
   ret void
 }
 
-
 ; The GEP is present after the aliasing store, preventing to move the memcpy before
 ; (without further analysis/transformation)
 define void @copyaliaswithproducerinbetween(%S* %src, %S* %dst) {
-; CHECK-LABEL: copyalias
+; CHECK-LABEL: copyaliaswithproducerinbetween
 ; CHECK-NEXT: [[LOAD:%[a-z0-9\.]+]] = load %S, %S* %src
 ; CHECK-NOT: call
   %1 = load %S, %S* %src
   store %S undef, %S* %dst
   %dst2 = getelementptr %S , %S* %dst, i64 1
   store %S %1, %S* %dst2
   ret void
 }