[PATCH] Use 16 byte stack alignment for NaCl on ARM, and fix a varargs bug

Mark Seaborn mseaborn at chromium.org
Tue Feb 11 14:41:50 PST 2014


On 4 February 2014 00:20, Stepan Dyatkovskiy <stpworld at narod.ru> wrote:

> Hi all,
>
> Jan,
> Its proper fix, IMHO. But may be its better to check for alignments we
> exactly know we support? I mean:
> + if (NumGPRs && (Align == 8 || Align == 16)) && ...
>
> Otherwise you actually could just put the next line:
> + if (NumGPRs && (Align > 4)) && ...
>

OK, I've changed it to use "Align > 4".

Something stopped me from doing that last time. I need Renato and Chandler
> opinion.
> What strategy should be used here? "Align > 4" or "(Align == 8 || Align ==
> 16)"? Too small point details of course..
>
> Comments in method mentioned only 8 bytes alignment:
>     // Add padding for part of param recovered from GPRs, so
>     // its last byte must be at address K*8 - 1.
> That should be fixed with something like that:
> -    // its last byte must be at address K*8 - 1.
> +   // For example, if Align == 8, its last byte must be at address K*8 -
> 1.
>
> The same comments fix should be done in ARMMachineFunctionInfo.h, with
> "StByValParamsPadding" field.
>

OK, I've changed the comments as above.  I've updated
http://llvm-reviews.chandlerc.com/D2677.

I've also changed the commit message to reflect that this wasn't really a
bug, just a case that the code didn't handle because it didn't need to
(since before the change, only 4 and 8 byte alignments were supported).

Cheers,
Mark
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140211/76fbc4d3/attachment.html>


More information about the llvm-commits mailing list