This is an archive of the discontinued LLVM Phabricator instance.

ArgumentPromotion: Drop sret attribute on functions that are only called directly.
ClosedPublic

Authored by pcc on Jun 9 2015, 7:31 PM.

Details

Summary

If the first argument to a function is a 'this' argument and the second
has the sret attribute, the ArgumentPromotion pass may promote the 'this'
argument to more than one argument, violating the IR constraint that 'sret'
may only be applied to the first or second argument.

Although this IR constraint is arguably unnecessary, it highlighted the fact
that ArgPromotion does not need to preserve this attribute. Dropping the
attribute reduces register pressure in the backend by avoiding the register
copy required by sret. Because sret implies noalias, we also replace the
former with the latter.

Diff Detail

Event Timeline

pcc updated this revision to Diff 27423.Jun 9 2015, 7:31 PM
pcc retitled this revision from to ArgumentPromotion: Avoid invalidating the IR if a function's second argument is sret..
pcc updated this object.
pcc edited the test plan for this revision. (Show Details)
pcc added a reviewer: rnk.
pcc added a subscriber: Unknown Object (MLST).
rnk edited edge metadata.Jun 10 2015, 10:57 AM

First, the limitation that sret be parameter 1 or 2 is artificial. At the time, I didn't see how it could appear on any other parameter and I tried to make the most minimal change. Now we have a reason to generalize to other parameters. That said, all the backends handle sret appearing on any parameter, because the first parameter can be split up into multiple MVTs. Consider passing i64 as the first parameter on a 32-bit platform. So, we could lift the verifier limitation.

Second, could argpromotion just drop sret in this case? sret does two things: it's equivalent to noalias, and it ensures that the sret pointer is returned in eax. If all uses of the function are internal, dropping sret is conservatively correct and reduces register pressure. We don't leverage that eax return because it would be an ABI break with old LLVMs, so we can replace sret with noalias without losing any power.

Finally, if you like having argpromotion canonicalize sret to be the first parameter, I guess that's also kind of nice, so we could continue on this way. :)

lib/IR/Function.cpp
157–158

Oops, that's just a bug.

pcc added a comment.Jun 10 2015, 11:42 AM

Second, could argpromotion just drop sret in this case? sret does two things: it's equivalent to noalias, and it ensures that the sret pointer is returned in eax. If all uses of the function are internal, dropping sret is conservatively correct and reduces register pressure. We don't leverage that eax return because it would be an ABI break with old LLVMs, so we can replace sret with noalias without losing any power.

I agree, that seems like a much better solution.

pcc updated this revision to Diff 27462.Jun 10 2015, 1:32 PM
pcc retitled this revision from ArgumentPromotion: Avoid invalidating the IR if a function's second argument is sret. to ArgumentPromotion: Drop sret attribute on functions that are only called directly..
pcc updated this object.
pcc edited edge metadata.

New patch

rnk accepted this revision.Jun 10 2015, 1:51 PM
rnk edited edge metadata.

lgtm

This might be an interesting change, since we'll basically remove sret from all directly called functions in LTO builds, even if we weren't going to do arg promotion. I'm pretty confident it's safe, though.

lib/Transforms/IPO/ArgumentPromotion.cpp
260–264

I think this is inefficient because we end up with lots of temporary attribute sets, but whatever. It's kind of an API deficiency. =/

The alternative would be to compute the set of parameters that are losing sret and feed that into DoPromotion, which is ugly.

This revision is now accepted and ready to land.Jun 10 2015, 1:51 PM
This revision was automatically updated to reflect the committed changes.