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

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Tue May 31 16:40:25 PDT 2016


> On May 31, 2016, at 4:30 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
> +Mehdi, who set the mandatory data layout.
> 
>> On May 27, 2016, at 7:52 AM, Jun Bum Lim <junbuml at codeaurora.org> wrote:
>> 
>> junbuml added a comment.
>> 
>> Thanks Quentin for the review. I will address your comments soon.
>> 
>> 
>> ================
>> Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:238
>> @@ +237,3 @@
>> +
>> +  if (MaxIntSize == 0)
>> +    MaxIntSize = 1;
>> ----------------
>> qcolombet wrote:
>>> junbuml wrote:
>>>> dberlin wrote:
>>>>> How and when can this happen?
>>>> If there is no information about LegalIntWidth extracted from datalayout, this could be 0.  If we remove the target datalayout in input IR, this will be 0.
>>> Like Daniel said, DL are mandatory now. If that may still happen, (int would be an illegal type??) add a comment on when this is the case and add a test case for that!
>> As far as I check, I didn't encounter any complain when injecting an IR without "target datalayout" into opt. Also there are many test cases without datalayout. It seems also possible to assign empty string like target datalayout="". Did the mandatory mean in IRs generated from the frontend? 
> 
> Mehdi, could you clarify that point?
> Moreover, shouldn’t the IR verifier complain if the data layout string is missing?

No data layout in the Textual IR means "use the default data layout" for the triple.
So the verifier can't really do anything about that: there is *always* a DataLayout available (last resort if the default-constructed one).

-- 
Mehdi


> 
>> 
>> If DL.getLargestLegalIntTypeSizeInBits() returns 0, I think we should bail out with the assumption that we don't have proper information about backend.
>> 
>> 
>> http://reviews.llvm.org/D20262



More information about the llvm-commits mailing list