[PATCH] Begin adding docs and IR-level support for the inalloca attribute
rnk at google.com
Wed Dec 18 18:00:49 PST 2013
On Tue, Dec 17, 2013 at 2:11 PM, Nick Lewycky <nlewycky at google.com> wrote:
> On 16 December 2013 14:03, Reid Kleckner <rnk at google.com> wrote:
> + The caller must pass in an alloca value into an ``inalloca``
> + parameter, and an alloca may be used as an ``inalloca`` argument at
> + most once. The ``inalloca`` attribute cannot be used in conjunction
> + with ``byval``, ``readonly``, and others. These rules are enforced
> + by the verifier.
> Why is inalloca forbidden in conjunction with readonly, and which others?
> Noalias? Nocapture? What about "nest"? "returned"?
OK, I'll make it a complete list. It's basically readonly plus the
standard list of ABI attributes: sret, nest, returned, inreg, and byval.
sret and returned could work, but are fairly nonsensical.
Readonly deserves it's own explanation.
> I realize that after a call the alloca must be assumed to have been
> clobbered, but what if the callee doesn't contain IR to perform said
> clobbering? An optimization would them go ahead and add the readonly
> attribute, and that would transform valid IR to invalid IR. I think this
> state should be permitted even though it indicates something nonsensical
> (this is not the only place in the IR we permit contradictory things just
> because we don't want to put the burden on the optimizer to check all
> possible cases.)
If I understand correctly, your desired end state is that optimizations
should be taught to avoid creating inalloca+readonly parameters, but the
verifier should not enforce this. Is that right?
My intention was to outlaw inalloca+readonly, change the verifier, and then
use that to find and fix optimizations doing this.
> + Assert1(!Attrs.hasAttribute(Idx, Attribute::ByVal) &&
> + !Attrs.hasAttribute(Idx, Attribute::InAlloca),
> + "Attribute 'byval' and 'inalloca' do not support unsized
> + V);
> "Attribute" --> "Attributes". Yes, I'm aware that pushes you over the 80
> column limit.
*shrug* clang-format says it's fine. :)
Based on this, I'm going to commit the LangRef change and send along
followups. We can reevaluate what we're doing with readonly when you're
back from vacation some time before the next release.
Thanks for the review!
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits