[llvm-commits] [PATCH] Adding x32 ABI triple and separate StackSlotSize in MCAsmInfo

[Evan Cheng]

On Jan 22, 2013, at 2:14 PM, Eli Bendersky <eliben at google.com> wrote:

> On Tue, Jan 22, 2013 at 12:56 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>> 
>> On Jan 22, 2013, at 9:26 AM, Eli Bendersky <eliben at google.com> wrote:
>> 
>>> On Tue, Jan 22, 2013 at 9:15 AM, Nadav Rotem <nrotem at apple.com> wrote:
>>>> Can we initialize StackSlotSize to zero and change getStackSlotSize to return PointerSize unless this value is overridden ?  This will allow us to keep most targets unmodified.
>>>> 
>>>> Otherwise LGTM.
>>>> 
>>>> 
>>> 
>>> Yes, I've actually considered this or a similar approach for more
>>> "defaultness". After some thinking it seemed to me that being explicit
>>> is more important here. While the changes to each target are trivial,
>>> they allow to see all the important values set in a single place (the
>>> MCAsmInfo subclass constructor), without requiring someone reading the
>>> code to hunt up in the base class.
>>> That said, if you think it's important to keep the targets untouched,
>>> I don't feel strongly about this and will revise the patch.
>> 
>> If that's the intention, then please change the name of the field. It's just going to confuse people.
>> 
> 
> Evan, I'm not sure what you're referring to here. Could you please clarify?

CalleeSaveStackSlotSize now has special meaning when it is zero, perhaps something like CalleeSaveStackSlotSizeOverride is more appropriate? I personally really don't like this, I think it optimizing for out-of-tree targets is not the right approach. It's making the code harder to understand.

Evan

> 
> Eli