[llvm-commits] [patch] Use correct alignment when merging zero sized arrays in instruction combine

[Richard Osborne]

On 17/09/12 18:47, Duncan Sands wrote:
> Hi Richard,
>
> On 17/09/12 17:50, Richard Osborne wrote:
>> I noticed that when merging zero sized allocas instruction combine 
>> ignores the
>> alignment set on the alloca instruction. This patch fixes this. OK to 
>> commit?
>
> thanks for spotting this.
>
> +/// getPointeeAlignment - Compute the minimum alignment of the given 
> alloca
> +/// instruction.
>
> ^ The function name and text in the comment don't match the actual 
> function:
>
> +static unsigned getAllocaAlignment(AllocaInst &AI, TargetData &TD) {
> +  return std::max(TD.getPrefTypeAlignment(AI.getAllocatedType()),
> +                  AI.getAlignment());
> +}
>
> While this logic is consistent with what was there before, I'm not 
> sure it
> is really right.  Consider a few lines before:
>
>     if (AI.getAlignment() == 0)
> AI.setAlignment(TD->getPrefTypeAlignment(AI.getAllocatedType()));
>
> So if the alloca was explicitly marked "align 1" then its alignment 
> doesn't
> get bumped up.  However in the merging we do bump it up.  To be 
> consistent
> the logic should probably be the following:
>   if EntryAI has alignment 0, then give it its preferred alignment;
>   give EntryAI the maximum of EntryAI.getAlignment() and 
> AI.getAlignment()
> (the last of these two is known non-zero already due to the code I 
> mentioned
> above).
>
> Ciao, Duncan.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Thanks for your comments. I've updated to patch to use the logic you 
suggested.

Regards,

Richard

-- 
Richard Osborne | XMOS
http://www.xmos.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: alloca_align.diff
Type: text/x-patch
Size: 2384 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120917/32b64eae/attachment.bin>