PATCH + test for PR18032 (InstCombine issue)

[Benjamin Kramer]

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