[PATCH] D26811: [MemCpyOpt] Don't sink LoadInst below possible clobber.

[Eli Friedman via llvm-commits]

efriedma requested changes to this revision.
efriedma added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Transforms/Scalar/MemCpyOptimizer.cpp:619
         if (P && P != SI) {
-          if (!moveUp(AA, SI, P))
+          if (!moveUp(AA, SI, P, LI))
             P = nullptr;
----------------
I'm not really following what this patch is doing.  Whether it's legal to sink the load to location P isn't related to whether it's legal to hoist the store to location P.

On a side-note, this whole block of code will never run in a normal pass pipeline; "T->isAggregateType()" will never be true because instcombine breaks up aggregate loads and stores.


================
Comment at: test/Transforms/MemCpyOpt/load-store-to-memcpy.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -basicaa -scoped-noalias -aa-eval -evaluate-aa-metadata -memcpyopt -S %s 2>/dev/null | FileCheck %s
+
----------------
-aa-eval doesn't do anything useful here.


Repository:
  rL LLVM

https://reviews.llvm.org/D26811