[PATCH] [fast-isel] x86_64 FastLowerArguments

[Chad Rosier]

On Feb 25, 2013, at 12:50 PM, Jan Voung <jvoung at chromium.org> wrote:

> On Mon, Feb 25, 2013 at 10:01 AM, Chad Rosier <mcrosier at apple.com> wrote:
> 
> On Feb 22, 2013, at 5:05 PM, Jan Voung <jvoung at chromium.org> wrote:
> 
>> +  // Only handle simple cases. i.e. Up to 6 i32/i64 scalar arguments.
>> +  unsigned Idx = 1;
>> +  for (Function::const_arg_iterator I = F->arg_begin(), E = F->arg_end();
>> +       I != E; ++I, ++Idx) {
>> +    if (Idx > 6)
>> +      return false;
>> +
>> +    if (F->getAttributes().hasAttribute(Idx, Attribute::InReg) ||
>> +        F->getAttributes().hasAttribute(Idx, Attribute::StructRet) ||
>> +        F->getAttributes().hasAttribute(Idx, Attribute::ByVal))
>> +      return false;
>> 
>> Do parameters with the nest attribute need to be checked too?  The CC_X86_64_C .td definition has special treatment for that, using r10 as the static chain register.
> 
> Yes, thanks.
> 
> I wonder if these multiple attribute checks get optimized to decent/fast code by the compiler.  Some of attributes can be pretty rare for x86-64 (e.g., InReg?)?

Bill, do you have any comment here since you've been doing a lot of work on the Attributes?  I assume the lookups aren't terribly slow, but also not super efficient.

Would it be beneficial to expose an API to allow multiple checks with a single call to hasAttributes?   E.g.,

  hasAttribute(Idx, Attribute::InReg, Attribute::StructReg, Attribute::ByVal);

>  
> 
>> Also, would it be worth adding a test that checks that N basic blocks were truly selected by fast isel, according to the "-stats" output?
> 
> I've added a new llc flag, -fast-isel-abort-args, which is the same as fast-isel-abort, but is for formal argument lowering only.
> 
> Revised patch attached.  Test case added that uses the new -fast-isel-abort-args option.
> 
> 
> Thanks, looks pretty good. 
>  
>  Thanks for the feedback, Jan.
> 
>  Chad
> 
> 
> 
>> 
>> 
>> 
>> On Fri, Feb 22, 2013 at 3:34 PM, Chad Rosier <mcrosier at apple.com> wrote:
>> All,
>> The attached patch implements a basic version of FastLowerArguments for x86 fast-isel.  This is analogous to Evan's commit r174855, but for x86 of course.  Please have a look (especially those familiar with x86 calling conventions).
>> 
>>  Regards,
>>   Chad
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130225/b5aec17f/attachment.html>