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

[Jun Bum Lim via llvm-commits]

Hi Mehdi,

Do you expect  DL.getLargestLegalIntTypeSizeInBits() always return non-zero?
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