[PATCH] D20262: [DSE]Split memset when the memset is small enough to be lowered to stores

[Mehdi Amini via llvm-commits]

> On Jun 1, 2016, at 11:02 AM, Jun Bum Lim <junbuml at codeaurora.org> wrote:
> 
> Hi Mehdi,
> 
> Do you expect  DL.getLargestLegalIntTypeSizeInBits() always return non-zero?

So I checked the code (and doxygen) and indeed it is expected to be an optional information on the DataLayout.
The default data-layout won't specify it, and clients should handle the zero case.


-- 
Mehdi


> However, using the test case added in this patch, we can easily make
> DL.getLargestLegalIntTypeSizeInBits() return  0 whenever we remove
> datalayout in the IR below.  Please correct me if I miss something here. 
> 
> ;  RUN: opt < %s -basicaa -dse -mtriple=arm64-linux-gnu -S 
> 
> ; target datalayout = "e-m:e-i64:64-i128:128-n32:64-S128"
> 
> define void @test_32_8to15(i64* nocapture %P, i64 %n64) #0 {
> entry:
> %0 = bitcast i64* %P to i8*
> call void @llvm.memset.p0i8.i64(i8* %0, i8 0, i64 32, i32 8, i1 false)
> %arrayidx1 = getelementptr inbounds i64, i64* %P, i64  1  store i64 %n64,
> i64* %arrayidx1  ret void }
> 
> 
> Thanks,
> Jun
> 
> 
> -----Original Message-----
> From: mehdi.amini at apple.com [mailto:mehdi.amini at apple.com] 
> Sent: Wednesday, June 01, 2016 1:26 PM
> To: reviews+D20262+public+d72d68e64d18c977 at reviews.llvm.org
> Cc: junbuml at codeaurora.org; hfinkel at anl.gov; eeckstein at apple.com;
> mcrosier at codeaurora.org; dberlin at dberlin.org; qcolombet at apple.com;
> llvm-commits at lists.llvm.org
> Subject: Re: [PATCH] D20262: [DSE]Split memset when the memset is small
> enough to be lowered to stores
> 
> 
>> On Jun 1, 2016, at 8:47 AM, Jun Bum Lim <junbuml at codeaurora.org> wrote:
>> 
>> junbuml added a comment.
>> 
>>> http://llvm.org/docs/doxygen/html/Module_8cpp_source.html#l00375
>> 
>>> "const DataLayout &Module::getDataLayout() const { return DL; }"
>> 
>>> It returns a reference, so it is guaranteed to not be null.
>> 
>>> "No data layout in the Textual IR means "use the default data layout" for
> the triple."
>> 
>> 
>> Thanks for clarifying this. So, we are guaranteed to DL even without
> datalayout in the textual IR. However, this doesn't mean that we can always
> get the non-zero from DL.getLargestLegalIntTypeSizeInBits().  As I test, we
> can use an empty datalayout in IR like : target datalayout="".
> 
> I feel you are speculating, and haven't really tried the effects of what
> your describing, neither look at the code that constructs a DataLayout.
> Writing this in textual IR is identical to not write anything.
> 
> 
>> 
>>> Wouldn't DL.getLargestLegalIntTypeSizeInBits() == 0 imply that you can
> have icmp or select? Hence almost nothing in llvm would work?
>> 
>> 
>> Although we can make DL.getLargestLegalIntTypeSizeInBits() return  0,
> 
> Please write a test case.
> 
> 
>> I also doubt when this is meaningful. If this doesn't make sense, I 
>> will add  assert in getLargestLegalIntTypeSizeInBits().
> 
> I don't think such an assert makes sense here, on a function that is
> frequently called.
> If you insist on verifying properties on the DataLayout, do it in reset().
> 
>> Can anyone give us any feedback if there is any situation where we need
> to consider getLargestLegalIntTypeSizeInBits() == 0 ?  As far as I test, I
> didn't see any unit test failure with the assert in
> getLargestLegalIntTypeSizeInBits() == 0.
> 
> Not surprising...
> 
> 
> --
> Mehdi
> 
>