Page MenuHomePhabricator

[IR] Set nocapture attribute for va_start, va_end, and va_copy
Needs ReviewPublic

Authored by eush on Nov 29 2018, 9:47 AM.

Details

Summary

This adds nocapture attribute to the definitions of intrinsics va_start,
va_end, and va_copy, which indicates that they do not make any copies of
pointers to va_list that outlive the call.

This allows TailCallElim to mark tail calls in variadic functions as such, and
targets to successfully eliminate these calls if it's otherwise possible.

Currently unmarked tail calls in variadic functions are not recognized because
TailCallElim conservatively extends the lifetime of va_list local to the end
of the function.

Diff Detail

Event Timeline

eush created this revision.Nov 29 2018, 9:47 AM
RKSimon resigned from this revision.Dec 3 2018, 12:21 AM

Sorry @eush, but I don't know enough about this to review.

As far as I know, for all in-tree targets, this should be fine... and for all ABIs I know of in common use. I'm a little nervous that it will eventually break some obscure platform where a va_list can somehow point to itself, though. (At least, an implementation like that seems plausible.)

On a related note, I'm a little concerned that this missed optimization is actually covering up a bug: markTails() in TailRecursionElimination currently doesn't model the fact that on many targets, va_lists point to memory that's implicitly allocated on the stack, so it adds incorrect "tail" markings to certain calls. (This is technically orthogonal, but it's much more likely to come up with this patch because people usually declare va_list as a local variable.)

Is this patch still being reviewed?

Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: jdoerfert. · View Herald Transcript