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

[Nick Lewycky]

Stephen Lin wrote:
> Hi Nick,
>
> Here is the patch that added the attribute:
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130415/171972.html
> .

Thank you! The things that spring to mind may have already been 
addressed on that thread, let me go read it.

> 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.

Okay, so I haven't read the thread yet. I will. :) But my immediate 
reaction is that this sounds like a calling convention.

Suppose we manage to internalize the constructor (ie., all callers are 
visible), then the calling convention can be changed to fastcc which 
means "llvm can do anything it wants, even different things for 
different functions". If there's an optimization which we could be 
doing, but that fastcc isn't doing it then we ought to fix that! That 
will apply to many more functions than just constructors and more 
architectures than arm.

If we don't internalize it, we'll see the 'armthisreturn' calling 
convention and the caller and callee both know that the argument is 
returned. But notice that in this case, it isn't possible for 
deadargelim to remove the return type, since we don't have all the 
callers, so deadargelim must conservatively assume the return value is used.

> 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.

What about implicit constructors or other empty constructors? Normally 
those will get inlined, but supposing that inlining is disabled (for 
instance, to prevent code size growth which I hear is important on ARM) 
we'd like deadargelim to be able to nuke the argument and return value 
together. I think that should be a testcase.

> 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!

There's an elephant you never mentioned in this email: using the 
'returned' attribute for IR-level optimizations. What things are 
possible there? If the answer really is "nothing" then I'm going to 
press that this doesn't belong and should be a calling convention. If 
there are cool things we can do with it at the IR-optimizing level, then 
that may be enough to justify its existence ABI issues aside.

Nick

> 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
>>
>>
>