[PATCH] D42974: [SROA] Take advantage of separate alignments for memcpy source and destination

[Daniel Neilson via Phabricator via llvm-commits]

dneilson added a comment.

Thank you for taking a look at this Eli! Very much appreciated



================
Comment at: lib/Transforms/Scalar/SROA.cpp:2701
       II.setDest(getNewAllocaSlicePtr(IRB, OldPtr->getType()));
-      II.setAlignment(getSliceAlign());
+      II.setDestAlignment(getSliceAlign());
 
----------------
efriedma wrote:
> Testcase?
Any number of the memsets in tests/Transforms/SROA/basictest.ll act as a test for this. For example, the memset in test18 at line 783.


================
Comment at: lib/Transforms/Scalar/SROA.cpp:2828
         II.setSource(AdjustedPtr);
-
-      if (II.getAlignment() > SliceAlign) {
-        II.setAlignment(MinAlign(II.getAlignment(), SliceAlign));
+        II.setSourceAlignment(SliceAlign);
       }
----------------
efriedma wrote:
> Testcase?  (At least, at first glance it doesn't look like the memcpy tests cover this.)
test18 -- the memcpy at line 798 tests the IsDest case
test18 -- the memcpy at line 803 tests the !IsDest case

There are others as well; this is just a sampling.


================
Comment at: lib/Transforms/Scalar/SROA.cpp:2899
+          IsDest ? OtherPtr : OurPtr, IsDest ? OtherAlign : SliceAlign,
+          Size, II.isVolatile());
       if (AATags)
----------------
efriedma wrote:
> All the checks for IsDest here are confusing... could you make explicit variables SrcPtr/DstPtr/SrcAlign/DstAlign?
Fair point.. I completely agree. I'll get a revision up shortly.


Repository:
  rL LLVM

https://reviews.llvm.org/D42974