On Jun 7, 2013, at 4:42 PM, Arthur O'Dwyer <[email protected]> wrote:
> On Fri, Jun 7, 2013 at 3:22 PM, John McCall <[email protected]> wrote:
>> On Jun 7, 2013, at 12:07 PM, Arthur O'Dwyer <[email protected]> 
>> wrote:
>>> 
>>> +VALUE_CODEGENOPT(PassSmallStructsConvention, 2, 0) /// If
>>> -fpcc-struct-return (==1) or -freg-struct-return (==2) is specified.
>> 
>> Use ENUM_CODEGENOPT instead of magic numbers, please.
> 
> Fixed. I renamed this variable to SmallStructsConvention and the enum to
> SmallStructsConventionKind to save a bit of horizontal space.
> I removed the "default: assert(false);" case in my helper function, as it was
> now triggering a warning from -Wcovered-switch-default.

How about "StructReturnConventionKind"?

>> "Override the default ABI to encourage small structs to be returned in 
>> registers"
> 
> I strongly prefer not to use the word "encourage", as that makes it sound like
> it's an optional optimization or something. Really it changes the ABI so that
> small structs WILL be returned in registers, full stop. See my proposed 
> revised
> wording below.

Sounds reasonable.
> 
>> Please error out if the target is not i386, since you've only added
>> support for the switch on that target.
> 
> Fixed, I believe.
> clang: error: unsupported option '-fpcc-struct-return' for target
> 'x86_64-apple-darwin12.3.0'

Okay.

Please add tests and then attach the final patch instead of including it inline.

John.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to