[llvm-commits] sret-demotion for large struct returns working on x86!

Kenneth Uildriks kennethuil at gmail.com
Mon Nov 9 13:43:02 PST 2009


On Mon, Nov 9, 2009 at 3:14 PM, Dan Gohman <gohman at apple.com> wrote:
>
> On Nov 8, 2009, at 6:37 PM, Kenneth Uildriks wrote:
>
>> If a struct can't be returned in registers, have SelectionDAGBuild
>> insert a hidden sret parameter and have return instructions store the
>> return value through the hidden pointer.  Formal arguments, return
>> instructions, and calls are all updated as needed.
>>
>> Only x86 has the code to actually check whether sret-demotion is
>> needed.... other platforms invariably report that they can return
>> values in registers, even when they can't.
>
> Hello,
>
> Overall this patch looks good. Thanks for working on this! I
> have a few comments.
>
>> +  // True if the function needs to be sret-demoted
>> +  bool CantLowerReturn;
>>
>> +  // Virtual register to hold the sret pointer for sret-demotion
>> +  unsigned DemoteRegister;
>
> Would it be possible to put this field in FunctionLoweringInfo
> instead of MachineFunction? It's information that's only really
> relevant during lowering to the SelectionDAG.

Thanks for pointing that out... I found code in SelectionDAGBuild that
pulls the FunctionLoweringInfo from the DAG.  Assuming I always get
the same instance back for the same function, it should be pretty
easy.

>
>> -    if (F->paramHasAttr(0, Attribute::SExt))
>> -      ExtendKind = ISD::SIGN_EXTEND;
>> -    else if (F->paramHasAttr(0, Attribute::ZExt))
>> -      ExtendKind = ISD::ZERO_EXTEND;
>
> I'm unclear on why this code is being removed. Can you explain
> what's happening here?

I was assuming that the attributes only appeared on int return values,
not struct return values.  However, "getReturnInfo" is declared to be
more general than the use to which I'm currently putting it, and that
if it's called with an int return type without the attribute, it'll
return the wrong EVT for it.

I'll put them back in and pass the flags for those attributes.

>
>> +  if (!CanLowerReturn)
>> +  {
>
> In general, LLVM style has the opening brace on the same line as the
> if, else, for, etc.
>
> Dan
>
>

Whoops.




More information about the llvm-commits mailing list