[cfe-commits] r143596 - /cfe/trunk/lib/CodeGen/TargetInfo.cpp

Hatanaka, Akira ahatanaka at mips.com
Thu Jan 5 12:47:34 PST 2012


Please review the attached patch.

I added comments and changed the type of ABIArgInfo's padding to llvm::Type*.
 
________________________________________
From: Eli Friedman [eli.friedman at gmail.com]
Sent: Wednesday, January 04, 2012 4:10 PM
To: Hatanaka, Akira
Cc: cfe-commits at cs.uiuc.edu
Subject: Re: [cfe-commits] r143596 - /cfe/trunk/lib/CodeGen/TargetInfo.cpp

On Wed, Jan 4, 2012 at 11:21 AM, Hatanaka, Akira <ahatanaka at mips.com> wrote:
> So I finally came up with a way to fix the alignment issue.
> Please review the attached patch & test case.
>
> The idea is to add a parameter to ABIArgInfo::getDirect that is used to set the size of a padding inserted before a byval aggregate. The test case shows padding is inserted before the first element of a 16-byte aligned byval structure when the first element is not aligned to a 16-byte boundary.

The one conceptual issue that I have with this patch is that measuring
the padding in bytes isn't very general or intuitive; how many
registers is 8 bytes?  What if some target needs to insert a float of
padding?  I think if target-specific code decides it needs an i64 of
padding, it should put the llvm::Type "i64" into ABIArgInfo, etc.

Some more comments for the target-independent changes would be nice.
Otherwise, mechanically, the patch looks fine.

-Eli
-------------- next part --------------
A non-text attachment was scrubbed...
Name: padding2.patch
Type: text/x-patch
Size: 7544 bytes
Desc: padding2.patch
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120105/c16e65e1/attachment.bin>


More information about the cfe-commits mailing list