PATCH + test for PR18032 (InstCombine issue)

[Stepan Dyatkovskiy]

Hi Ben,

 > Why do you think this is an issue? The alignment of a NULL point is 
essentially
 > infinite, picking the largest possible value is odd but it shouldn't 
cause problems.

May be its not a bug. And obviously it is not. Though it is still not 
quite expected behaviour. If it's not a bug, can we close PR18032 issue 
then?

-Stepan.

Benjamin Kramer wrote:
>
> On 20.12.2013, at 12:45, Stepan Dyatkovskiy <stpworld at narod.ru> wrote:
>
>> Hi all,
>> Please find in attachment my PR18032 fix for review.
>>
>> Issue description:
>> InstCombine Pass emits wrong alignment for 'load' instruction with 'null' as
>> address.
>> Problem was in Local.cpp, llvm::getOrEnforceKnownAlignment method.
>> Method analyzes trailing zeroes in address values and suggest its own
>> aligment.
>> For example: for address 0x123123a8 we have 3 binary trailing zeroes, and
>> thus, method will suggest you 8 bytes alignment.
>> Doing same logic, it will suggest you maximum possible alignment (2^29) for
>> 'null' pointer.
>> Then InstCombiner::visitLoadInst method will select maximum alignment of what
>> was given in IR and what was suggested by getOrEnforceKnownAlignment; these
>> strings:
>>    if (KnownAlign > EffectiveLoadAlign)
>>      LI.setAlignment(KnownAlign);
>> So, finally we got align of 536870912 bytes:
>
> Why do you think this is an issue? The alignment of a NULL point is essentially
> infinite, picking the largest possible value is odd but it shouldn't cause problems.
> Special casing NULL doesn't make sense to me either, you'll still get a ridiculously
> large alignment value e.g. for a constant 0x80000000 pointer.
>
> When I compile the test case from PR18032 with clang/LLVM trunk I get
>
> 	movq	%gs:0, %rax
> 	movq	8(%rax), %rsi
>
> which looks right to me, maybe it was an issue in 3.3 that has been fixed already?
>
> - Ben
>
>>
>> The fix:
>> Add one more special case for getOrEnforceKnownAlignment. If address is 0,
>> we don't need to anylize trailing zeroes,
>> just return alignment known already (PrefAlign value).
>>
>> -Stepan
>> <pr18032-2013-12-20.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>