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

[Daniel Neilson via Phabricator via llvm-commits]

dneilson added inline comments.


================
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:
> dneilson wrote:
> > 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.
> Okay.  It would be nice to have a test which explicitly checks we preserve the alignment on the other operand.
Those same memcpys end up testing both; that's why the test case change in test18 is as it is in this patch. I'll annotate the lines in the test below...


================
Comment at: test/Transforms/SROA/basictest.ll:83
   %b = getelementptr [300 x i8], [300 x i8]* %a, i64 0, i64 0
-  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %b, i8* %src, i32 300, i1 false), !tbaa !0
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %b, i8* align 8 %src, i32 300, i1 false), !tbaa !0
 ; CHECK-NEXT: %[[gep_dst:.*]] = getelementptr inbounds [42 x i8], [42 x i8]* %[[test3_a1]], i64 0, i64 0
----------------
FYI - the alignment on the %src arg here was changed to force some variety in the alignments on memcpys that are created in this test.


================
Comment at: test/Transforms/SROA/basictest.ll:781
 ; CHECK-NEXT: %[[agep1:.*]] = getelementptr inbounds [34 x i8], [34 x i8]* %[[a]], i64 0, i64 0
-; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i32(i8* %[[agep1]], i8* %src, i32 %size, {{.*}}), !tbaa [[TAG_3]]
+; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 1 %[[agep1]], i8* %src, i32 %size, {{.*}}), !tbaa [[TAG_3]]
 ; CHECK-NEXT: %[[agep2:.*]] = getelementptr inbounds [34 x i8], [34 x i8]* %[[a]], i64 0, i64 0
----------------
This is the IsDest case for !IsSplittable


================
Comment at: test/Transforms/SROA/basictest.ll:790
 ; CHECK-NEXT: %[[agep3:.*]] = getelementptr inbounds [34 x i8], [34 x i8]* %[[a]], i64 0, i64 0
-; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dst, i8* %[[agep3]], i32 %size, {{.*}}), !tbaa [[TAG_11]]
+; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dst, i8* align 1 %[[agep3]], i32 %size, {{.*}}), !tbaa [[TAG_11]]
 ; CHECK-NEXT: ret void
----------------
This is the !IsDest case for !IsSplittable


Repository:
  rL LLVM

https://reviews.llvm.org/D42974