ARM, PR15868 Release-blocker fix. ByVal parameters padding fix.

[Manman Ren]

On May 13, 2013, at 1:28 PM, Evan Cheng wrote:

> Hi Stepan,
> 
> Your explanation below seems to indicate this is only an issue when stack alignment is 8 (or smaller?). But I don't see check for stack alignment anywhere in the code.
  unsigned Align = MF.getTarget().getFrameLowering()->getStackAlignment();
  ArgRegsSize = NumGPRs * 4;
  ArgRegsSaveSize = (ArgRegsSize + Align - 1) & ~(Align - 1);
The ArgRegsSaveSize is different from ArgRegsSize when stack alignment is 8 or larger.
And padding is added when they differ.

Thanks,
Manman
> 
> Other nitpicks:
> +    // If parameter was splitted between stack and GPRs...
> Typo: s/splitted/split
> 
> -    return MFI->CreateFixedObject(4, ArgOffset, !ForceMutable);
> +    return MFI->CreateFixedObject(
> +        4, AFI->getStoredByValParamsPadding() + ArgOffset, !ForceMutable);
> 
> You should just use Padding in place of AFI->getStoredByValParamsPadding() here.
> 
> +  /// StByValParamsPadding - in when we store params in memory
> +  /// sometimes we need to insert gap before parameter start address.
> +  ///
> 
> The comment doesn't parse. "in when"?
> 
> Evan
> 
> On May 13, 2013, at 6:26 AM, Stepan Dyatkovskiy <STPWORLD at narod.ru> wrote:
> 
>> Hi all,
>> 
>> In case when stack alignment is 8 and GPRs parameter part size is not N*8: we add padding to GPRs part, so part's last byte must be recovered at address K*8-1.
>> We need to do it, since remained (stack) part of parameter starts from address K*8, and we need to "attach" "GPRs head" without gaps to it:
>> Stack:
>> |---- 8 bytes block ----| |---- 8 bytes block ----| |---- 8 bytes...
>> [ [padding] [GPRs head] ] [ ------ Tail passed via stack  ------ ...
>> 
>> Note, once we added padding we need to correct *all* Arg offsets that are going after padded one.
>> That's why we need this fix: Arg offsets were never corrected before this patch. See new test-case included in patch.
>> 
>> We also don't need to insert padding for byval parameters that are stored in GPRs only. We need pad only last byval parameter and only in case it outsides GPRs and stack alignment = 8.
>> 
>> Please find the patch in attachment.
>> 
>> This patch reduces stack usage for some cases:
>> We can reduce ArgRegsSaveArea since inner 4 bytes sized byval params my be "packed" with alignment 4.
>> 
>> This patch also fixes PR15868 release-blocking issue.
>> 
>> -Stepan.
>> 
>> <pr15868-2013-05-13-2.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits