[PATCH] Teach DeadArgElimination not to eliminate return values of functions with 'returned' arguments

[Stephen Lin]

Hi Nick,

Here is the patch that added the attribute:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130415/171972.html
.

In the cases where 'returned' will be generated by the front-end
immediately, which is ARM C++ 'this'-returning constructors and
destructors, 'returned' generally allows both the caller and callee to
generate better code, because the the first parameter (for passing
'this') and the return value are both in same register. So to return
'this', the constructor or destructor doesn't have to do anything
extra to return 'this': it just has to not touch with register. On the
other hand, the caller can use the information of the 'this' return to
elide save/restoring the 'this' pointer across the call, reducing a
few instructions. 'this'-returning constructors and destructors were
explicitly added to the ABI for this optimization purpose.

Admittedly, there are situations where it would be better to drop the
return value. For example, if the argument and return value are both
superfluous (i.e. the constructor/destructor doesn't do anything with
the 'this' pointer but pass it straight through) and the caller isn't
going to have the 'this' pointer of the callee handy in a register
anyway, then it would be better to get rid of both; however, it is
rare for a constructor or destructor to never use 'this'. Furthermore,
if there's significant register pressure in either the caller or the
callee, then maybe 'this' is going to have to be save/restored on both
sides anyway, and dropping the return value will keep it from having
to be restored on the callee side before being returned, etc.

Unfortunately, as far as I can tell, there's not really any way for an
IR-level pass to detect these kinds of situations without lots of
target and backend-implementation specific details, and, since for
now, the attribute is only being used in a very specific situation
where (I believe) it's usually more profitable to allow code
generation to take advantage of the extra information provided by
'returned', I decided to go with the default of leaving it in always.

That said, if you think there's a better approach to this that's more
robust and retains the optimization benefits for the ARM C++ ABI,
please let me know!

Stephen

On Fri, Jun 21, 2013 at 2:17 AM, Nick Lewycky <nicholas at mxc.ca> wrote:
> I'm sorry, I seem to have missed the discussion that led up to the
> 'returned' attribute being added to llvm ir. Could you please point me to
> that?
>
> I've read the comment in the patch and I'm not sure I'm convinced. Offhand,
> it seems that the correct thing to do is to remove the 'returned' attribute
> if we change the return type of the function to void. The fact we can change
> the function that way is implies that the ABI arguments are moot since we
> have control over the function's calling convention, since we control the
> function and all its callsites.
>
> Nick
>
> Stephen Lin wrote:
>>
>> Hi,
>>
>> This patch makes the DeadArgElimination IPO pass treat any return
>> value of any function with a 'returned' argument as live even if there
>> are no uses. This is meant to allow the clang patch committed as
>> revision r184205 (and subsequent minor fixes) to be recommitted after
>> being reverted in revision r184205 due to broken builds on ARM when
>> compiling with LTO.
>>
>> The 'returned' attribute is relatively new and indicates, as an
>> optimization hint to the caller, that a function always returns the
>> value of an argument as its return value. In the intended use cases,
>> it is almost always better to leave the return value on the function
>> than to take it off, because it's essentially free. (Specifically, for
>> a 'this'-returning constructor/destructor in the ARM C++ ABI using the
>> ARM calling convention, the 'this'-argument and the return value as in
>> the same register so the caller and callee basically can both leave it
>> untouched unless there is significant register pressure.)
>>
>> The patch includes comments describing the rationale for retaining the
>> return value in these cases and also includes a note that this should
>> be reinvestigated if 'returned' is applied more liberally in the
>> future.
>>
>> Any and all feedback is highly appreciated.
>>
>> Stephen
>>
>> <rdar://problem/14209661>
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>