[llvm] r235258 - [MemCpyOpt] Promote both memset/memcpy sizes if differently typed.

[Ahmed Bougacha]

On Sat, Apr 18, 2015 at 1:50 PM, David Majnemer
<david.majnemer at gmail.com> wrote:
>
>
> On Sat, Apr 18, 2015 at 6:57 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com>
> wrote:
>>
>> Author: ab
>> Date: Sat Apr 18 12:57:41 2015
>> New Revision: 235258
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=235258&view=rev
>> Log:
>> [MemCpyOpt] Promote both memset/memcpy sizes if differently typed.
>>
>> Followup to r235232, which caused PR23278.
>>
>> We can't assume the memset and memcpy sizes have the same type, as
>> nothing in the language reference prevents that.
>> Instead, zext both to i64 if they disagree.
>>
>> While there, robustify tests by using i8 %c rather than i8 0 for the
>> memset character.
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
>>     llvm/trunk/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp?rev=235258&r1=235257&r2=235258&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp Sat Apr 18
>> 12:57:41 2015
>> @@ -876,6 +876,12 @@ bool MemCpyOpt::processMemSetMemCpyDepen
>>
>>    IRBuilder<> Builder(MemCpy->getNextNode());
>>
>> +  // If the sizes have different types (i32 vs i64), promote both to i64.
>> +  if (DestSize->getType() != SrcSize->getType()) {
>> +    DestSize = Builder.CreateZExt(DestSize, Builder.getInt64Ty());
>> +    SrcSize = Builder.CreateZExt(SrcSize, Builder.getInt64Ty());
>> +  }
>> +
>
>
> I don't think this is quite right, I can crash memcpyopt with:
> define void @test_different_types_i32_i128(i8* %dst, i8* %src, i32
> %dst_size, i128 %src_size, i8 %c) {
>   call void @llvm.memset.p0i8.i32(i8* %dst, i8 %c, i32 %dst_size, i32 1, i1
> false)
>   call void @llvm.memcpy.p0i8.p0i8.i128(i8* %dst, i8* %src, i128 %src_size,
> i32 1, i1 false)
>   ret void
> }
>
> declare void @llvm.memcpy.p0i8.p0i8.i128(i8* nocapture, i8* nocapture
> readonly, i128, i32, i1)
> declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1)
>
> 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.

Ah, you're right, I confused myself with the examples.  r235261

Thanks!
-Ahmed

>
>>
>>    Value *MemsetLen =
>>        Builder.CreateSelect(Builder.CreateICmpULE(DestSize, SrcSize),
>>
>> ConstantInt::getNullValue(DestSize->getType()),
>>
>> Modified:
>> llvm/trunk/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll?rev=235258&r1=235257&r2=235258&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll
>> (original)
>> +++ llvm/trunk/test/Transforms/MemCpyOpt/memset-memcpy-redundant-memset.ll
>> Sat Apr 18 12:57:41 2015
>> @@ -8,14 +8,44 @@ target datalayout = "e-m:o-i64:64-f80:12
>>  ; CHECK-DAG: [[ULE:%[0-9]+]] = icmp ule i64 %dst_size, %src_size
>>  ; CHECK-DAG: [[SIZEDIFF:%[0-9]+]] = sub i64 %dst_size, %src_size
>>  ; CHECK-DAG: [[SIZE:%[0-9]+]] = select i1 [[ULE]], i64 0, i64
>> [[SIZEDIFF]]
>> -; CHECK-NEXT: call void @llvm.memset.p0i8.i64(i8* [[DST]], i8 0, i64
>> [[SIZE]], i32 1, i1 false)
>> +; CHECK-NEXT: call void @llvm.memset.p0i8.i64(i8* [[DST]], i8 %c, i64
>> [[SIZE]], i32 1, i1 false)
>>  ; CHECK-NEXT: ret void
>> -define void @test(i8* %src, i64 %src_size, i8* %dst, i64 %dst_size) {
>> -  call void @llvm.memset.p0i8.i64(i8* %dst, i8 0, i64 %dst_size, i32 1,
>> i1 false)
>> +define void @test(i8* %src, i64 %src_size, i8* %dst, i64 %dst_size, i8
>> %c) {
>> +  call void @llvm.memset.p0i8.i64(i8* %dst, i8 %c, i64 %dst_size, i32 1,
>> i1 false)
>> +  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %src_size,
>> i32 1, i1 false)
>> +  ret void
>> +}
>> +
>> +; CHECK-LABEL: define void @test_different_types_i32_i64
>> +; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src,
>> i64 %src_size, i32 1, i1 false)
>> +; CHECK-DAG: [[DSTSIZE:%[0-9]+]] = zext i32 %dst_size to i64
>> +; CHECK-DAG: [[DST:%[0-9]+]] = getelementptr i8, i8* %dst, i64 %src_size
>> +; CHECK-DAG: [[ULE:%[0-9]+]] = icmp ule i64 [[DSTSIZE]], %src_size
>> +; CHECK-DAG: [[SIZEDIFF:%[0-9]+]] = sub i64 [[DSTSIZE]], %src_size
>> +; CHECK-DAG: [[SIZE:%[0-9]+]] = select i1 [[ULE]], i64 0, i64
>> [[SIZEDIFF]]
>> +; CHECK-NEXT: call void @llvm.memset.p0i8.i64(i8* [[DST]], i8 %c, i64
>> [[SIZE]], i32 1, i1 false)
>> +; CHECK-NEXT: ret void
>> +define void @test_different_types_i32_i64(i8* %dst, i8* %src, i32
>> %dst_size, i64 %src_size, i8 %c) {
>> +  call void @llvm.memset.p0i8.i32(i8* %dst, i8 %c, i32 %dst_size, i32 1,
>> i1 false)
>>    call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %src_size,
>> i32 1, i1 false)
>>    ret void
>>  }
>>
>> +; CHECK-LABEL: define void @test_different_types_i64_i32
>> +; CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dst, i8* %src,
>> i32 %src_size, i32 1, i1 false)
>> +; CHECK-DAG: [[SRCSIZE:%[0-9]+]] = zext i32 %src_size to i64
>> +; CHECK-DAG: [[DST:%[0-9]+]] = getelementptr i8, i8* %dst, i64
>> [[SRCSIZE]]
>> +; CHECK-DAG: [[ULE:%[0-9]+]] = icmp ule i64 %dst_size, [[SRCSIZE]]
>> +; CHECK-DAG: [[SIZEDIFF:%[0-9]+]] = sub i64 %dst_size, [[SRCSIZE]]
>> +; CHECK-DAG: [[SIZE:%[0-9]+]] = select i1 [[ULE]], i64 0, i64
>> [[SIZEDIFF]]
>> +; CHECK-NEXT: call void @llvm.memset.p0i8.i64(i8* [[DST]], i8 %c, i64
>> [[SIZE]], i32 1, i1 false)
>> +; CHECK-NEXT: ret void
>> +define void @test_different_types_i64_i32(i8* %dst, i8* %src, i64
>> %dst_size, i32 %src_size, i8 %c) {
>> +  call void @llvm.memset.p0i8.i64(i8* %dst, i8 %c, i64 %dst_size, i32 1,
>> i1 false)
>> +  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dst, i8* %src, i32 %src_size,
>> i32 1, i1 false)
>> +  ret void
>> +}
>> +
>>  ; CHECK-LABEL: define void @test_align_same
>>  ; CHECK: call void @llvm.memset.p0i8.i64(i8* {{.*}}, i8 0, i64 {{.*}},
>> i32 8, i1 false)
>>  define void @test_align_same(i8* %src, i8* %dst, i64 %dst_size) {
>> @@ -52,3 +82,5 @@ define void @test_different_dst(i8* %dst
>>
>>  declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i32, i1)
>>  declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture
>> readonly, i64, i32, i1)
>> +declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1)
>> +declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture
>> readonly, i32, i32, i1)
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>