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

[Stephen Lin]

On Mon, Jun 24, 2013 at 2:38 PM, Stephen Lin <swlin at post.harvard.edu> wrote:
> On Mon, Jun 24, 2013 at 11:23 AM, Stephen Lin <swlin at post.harvard.edu> wrote:
>>> No, I don't have any alternatives, other than interprocedural coordination
>>> of code generation. Do you have any alternatives?
>>
>> The only thing I can think of is a few more heuristics to avoid
>> keeping the return value in cases where it is "obviously" not useful
>> to the caller.
>
> After some thought, my feeling is that some other pass (or passes),
> run before DeadArgumentElimination, should:
>
> 1. Infer 'returned' where it is implied by the callee's IR and looks
> like it could be useful in the caller(s) (based on some
> target-independent heuristics, which might not be 100% accurate but
> can make a reasonable guess)
> 2. Remove 'returned' if it's present but obviously not useful to any
> caller (note that this itself won't eliminate the return value, only
> the argument's 'returned' attribute, so wouldn't require the function
> to be internalized.)
> 3. Canonicalize uses of return values and uses of 'returned' arguments
> dominated by the call (in some manner TBD)
>
> In other words, 'returned' should be treated as a hint that should
> only be added by a pass if it has positive reason to believe that it
> will be useful, and can be removed if it looks like it will not be,
> rather than just being automatically applied automatically wherever
> the semantics hold. I think this is the best compromise approach that
> will allow the attribute to be taken advantage of without forcing
> inter-procedural coordination and complicated control flow analysis at
> code gen.
>
> Given that the 'returned' attribute will, for the near future, only
> generated by clang in a specific place, this doesn't seem necessary
> yet, but should be done eventually. In the meantime, I think the
> current patch is the best tactical approach to at least get the
> attribute working for ARM C++ ABI 'this'-returns (in particular,
> allowing elision of 'this' save/restores across constructor or
> destructor calls, which the current front-end only implementation does
> not do) without breaking.

Anyone still have a significant objection to the patch given the
above? I would like to get this in, if not...

Stephen