<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Apr 18, 2015 at 6:57 PM, Ahmed Bougacha <span dir="ltr"><<a href="mailto:ahmed.bougacha@gmail.com" target="_blank">ahmed.bougacha@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: ab<br>
Date: Sat Apr 18 12:57:41 2015<br>
New Revision: 235258<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=235258&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=235258&view=rev</a><br>
Log:<br>
[MemCpyOpt] Promote both memset/memcpy sizes if differently typed.<br>
<br>
Followup to r235232, which caused PR23278.<br>
<br>
We can't assume the memset and memcpy sizes have the same type, as<br>
nothing in the language reference prevents that.<br>
Instead, zext both to i64 if they disagree.<br>
<br>
While there, robustify tests by using i8 %c rather than i8 0 for the<br>
memset character.<br>
<br>
Modified:<br>
    llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp<br>
    llvm/trunk/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp?rev=235258&r1=235257&r2=235258&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp?rev=235258&r1=235257&r2=235258&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp Sat Apr 18 12:57:41 2015<br>
@@ -876,6 +876,12 @@ bool MemCpyOpt::processMemSetMemCpyDepen<br>
<br>
   IRBuilder<> Builder(MemCpy->getNextNode());<br>
<br>
+  // If the sizes have different types (i32 vs i64), promote both to i64.<br>
+  if (DestSize->getType() != SrcSize->getType()) {<br>
+    DestSize = Builder.CreateZExt(DestSize, Builder.getInt64Ty());<br>
+    SrcSize = Builder.CreateZExt(SrcSize, Builder.getInt64Ty());<br>
+  }<br>
+<br></blockquote><div><br></div><div>I don't think this is quite right, I can crash memcpyopt with:</div><div><div>define void @test_different_types_i32_i128(i8* %dst, i8* %src, i32 %dst_size, i128 %src_size, i8 %c) {</div><div>  call void @llvm.memset.p0i8.i32(i8* %dst, i8 %c, i32 %dst_size, i32 1, i1 false)</div><div>  call void @llvm.memcpy.p0i8.p0i8.i128(i8* %dst, i8* %src, i128 %src_size, i32 1, i1 false)</div><div>  ret void</div><div>}</div><div><br></div><div>declare void @llvm.memcpy.p0i8.p0i8.i128(i8* nocapture, i8* nocapture readonly, i128, i32, i1)</div><div>declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1)</div></div><div><br></div><div>I think you instead need to figure out which type is bigger and zext the smaller size to the larger type instead of choosing i64.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
   Value *MemsetLen =<br>
       Builder.CreateSelect(Builder.CreateICmpULE(DestSize, SrcSize),<br>
                            ConstantInt::getNullValue(DestSize->getType()),<br>
<br>
Modified: llvm/trunk/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll?rev=235258&r1=235257&r2=235258&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll?rev=235258&r1=235257&r2=235258&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll (original)<br>
+++ llvm/trunk/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll Sat Apr 18 12:57:41 2015<br>
@@ -8,14 +8,44 @@ target datalayout = "e-m:o-i64:64-f80:12<br>
 ; CHECK-DAG: [[ULE:%[0-9]+]] = icmp ule i64 %dst_size, %src_size<br>
 ; CHECK-DAG: [[SIZEDIFF:%[0-9]+]] = sub i64 %dst_size, %src_size<br>
 ; CHECK-DAG: [[SIZE:%[0-9]+]] = select i1 [[ULE]], i64 0, i64 [[SIZEDIFF]]<br>
-; CHECK-NEXT: call void @llvm.memset.p0i8.i64(i8* [[DST]], i8 0, i64 [[SIZE]], i32 1, i1 false)<br>
+; CHECK-NEXT: call void @llvm.memset.p0i8.i64(i8* [[DST]], i8 %c, i64 [[SIZE]], i32 1, i1 false)<br>
 ; CHECK-NEXT: ret void<br>
-define void @test(i8* %src, i64 %src_size, i8* %dst, i64 %dst_size) {<br>
-  call void @llvm.memset.p0i8.i64(i8* %dst, i8 0, i64 %dst_size, i32 1, i1 false)<br>
+define void @test(i8* %src, i64 %src_size, i8* %dst, i64 %dst_size, i8 %c) {<br>
+  call void @llvm.memset.p0i8.i64(i8* %dst, i8 %c, i64 %dst_size, i32 1, i1 false)<br>
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %src_size, i32 1, i1 false)<br>
+  ret void<br>
+}<br>
+<br>
+; CHECK-LABEL: define void @test_different_types_i32_i64<br>
+; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %src_size, i32 1, i1 false)<br>
+; CHECK-DAG: [[DSTSIZE:%[0-9]+]] = zext i32 %dst_size to i64<br>
+; CHECK-DAG: [[DST:%[0-9]+]] = getelementptr i8, i8* %dst, i64 %src_size<br>
+; CHECK-DAG: [[ULE:%[0-9]+]] = icmp ule i64 [[DSTSIZE]], %src_size<br>
+; CHECK-DAG: [[SIZEDIFF:%[0-9]+]] = sub i64 [[DSTSIZE]], %src_size<br>
+; CHECK-DAG: [[SIZE:%[0-9]+]] = select i1 [[ULE]], i64 0, i64 [[SIZEDIFF]]<br>
+; CHECK-NEXT: call void @llvm.memset.p0i8.i64(i8* [[DST]], i8 %c, i64 [[SIZE]], i32 1, i1 false)<br>
+; CHECK-NEXT: ret void<br>
+define void @test_different_types_i32_i64(i8* %dst, i8* %src, i32 %dst_size, i64 %src_size, i8 %c) {<br>
+  call void @llvm.memset.p0i8.i32(i8* %dst, i8 %c, i32 %dst_size, i32 1, i1 false)<br>
   call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %src_size, i32 1, i1 false)<br>
   ret void<br>
 }<br>
<br>
+; CHECK-LABEL: define void @test_different_types_i64_i32<br>
+; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dst, i8* %src, i32 %src_size, i32 1, i1 false)<br>
+; CHECK-DAG: [[SRCSIZE:%[0-9]+]] = zext i32 %src_size to i64<br>
+; CHECK-DAG: [[DST:%[0-9]+]] = getelementptr i8, i8* %dst, i64 [[SRCSIZE]]<br>
+; CHECK-DAG: [[ULE:%[0-9]+]] = icmp ule i64 %dst_size, [[SRCSIZE]]<br>
+; CHECK-DAG: [[SIZEDIFF:%[0-9]+]] = sub i64 %dst_size, [[SRCSIZE]]<br>
+; CHECK-DAG: [[SIZE:%[0-9]+]] = select i1 [[ULE]], i64 0, i64 [[SIZEDIFF]]<br>
+; CHECK-NEXT: call void @llvm.memset.p0i8.i64(i8* [[DST]], i8 %c, i64 [[SIZE]], i32 1, i1 false)<br>
+; CHECK-NEXT: ret void<br>
+define void @test_different_types_i64_i32(i8* %dst, i8* %src, i64 %dst_size, i32 %src_size, i8 %c) {<br>
+  call void @llvm.memset.p0i8.i64(i8* %dst, i8 %c, i64 %dst_size, i32 1, i1 false)<br>
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dst, i8* %src, i32 %src_size, i32 1, i1 false)<br>
+  ret void<br>
+}<br>
+<br>
 ; CHECK-LABEL: define void @test_align_same<br>
 ; CHECK: call void @llvm.memset.p0i8.i64(i8* {{.*}}, i8 0, i64 {{.*}}, i32 8, i1 false)<br>
 define void @test_align_same(i8* %src, i8* %dst, i64 %dst_size) {<br>
@@ -52,3 +82,5 @@ define void @test_different_dst(i8* %dst<br>
<br>
 declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i32, i1)<br>
 declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1)<br>
+declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1)<br>
+declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture readonly, i32, i32, i1)<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>