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

[Stephen Lin]

On Fri, Jun 21, 2013 at 10:07 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Fri, Jun 21, 2013 at 8:04 AM, Stephen Lin <swlin at post.harvard.edu> 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
>> .
>>
>> 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.
>
>
> Note that your argument could be reinterpreted as a proposal for an IR
> optimizer to add the "returned" attribute to constructors/destructors on
> x86.

It's not as likely to help on the x86 side (at least with standard
calling conventions) because the first argument and return value are
not the same register. And actually, we don't yet take advantage of
the 'returned' attribute fully in those cases anyway (it would require
non-trivial changes to the register allocator to do so.) The
optimization is fully supported in the ARM target though due to some
hacking by its LowerCall implementation.

>
> IMO, the presence of the returned attribute just doesn't seem like a very
> good heuristic for predicting whether the returned attribute should be used.
> I mean, sure, an IR optimizer can't predict what CodeGen will do with
> complete accuracy, but it shouldn't be hard to make a decent guess that's
> better than what the frontend is doing, given that the frontend has no idea
> what the callers or the callee actually look like.
>
> -Eli

Well, it's a pretty good heuristic now, given the very limited usage
of this attribute, but yes, I agree, it might not be in the future.

I would definitely be happy proactively putting in a better heuristic
now but I am not really sure how to do so. At the very minimum it
seems like it would require a lot of dataflow analysis that is not
currently done in DeadArgumentElimination.

Stephen