Constant merger patch to fix bug 17815 [Constant merging may illegally downgrade global alignment]

Chris Smowton chris at smowton.net
Wed Nov 6 03:40:04 PST 2013


On 05/11/13 15:59, Rafael EspĂ­ndola wrote:
> On 5 November 2013 06:12, Chris Smowton <chris at smowton.net> wrote:
>> Hi all,
>>
>> I attach a patch to fix the constant merging pass, which can sometimes
>> downgrade global alignment when one of those being merged has no specified
>> alignment (i.e. getAlignment() == 0), causing apparent null-pointer
>> segfaults that are actually x86 vector operation alignment faults.
>>
>> See http://llvm.org/bugs/show_bug.cgi?id=17815 for a test case and full
>> details. Patch is against 3.2 again I'm afraid due to time constraints, but
>> it doesn't look as though constmerge has changed significantly since then.
> Hi Chris,
> Thanks for working on this, it is quiet a bad bug.
>
> It looks like the optimization is simply not valid if only one of the
> constants has explicit alignment and we don't have TD, so we should
> avoid selecting this pair for replacement in the first place. Can you
> check if the attached patch solves the problem for you?
Agreed that it is invalid when dealing with implicit alignment without 
TD. I think you should also amend getAlignment as in my patch though -- 
as it stands, with TD it will always set to at least the preferred 
alignment, whereas if we're merging an "align 2" with an "align 4" we 
might as well use 4 rather than the preferred granularity.



More information about the llvm-commits mailing list