<div dir="ltr">+Chandler, Duncan, David M, and Reid (pseudorandom selection of people who might be interested/voiced opinions on prior threads/conversations on the subject)<br><br>Firstly, thanks Eddy for doing a lot of work in the opaque pointer area - it's greatly appreciated. This is a particularly thorny part of it (or, at least I think so at the moment - sounds like you've seen thornier parts further down the trail).<br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 19, 2016 at 2:47 PM, Eddy B. via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
In the past months, several options have been presented for making byval<br>
(and similar attributes, such as inalloca or sret) work with opaque pointers.<br>
<br>
The main two I've seen were byval(T) and byval(N) where N is the size of T.<br>
<br>
They both have their upsides and downsides, for example: byval(T) would be<br>
a type-parametric attribute, which, AFAIK, does not already exist and may<br>
complicate the attribute system significantly, while byval(N) would be hard<br>
to introduce in tests as computing N from T requires LLVM's DataLayout.<br></blockquote><div><br></div><div>By "hard to introduce in tests" you just mean it's hard to edit the test cases to add the correct value of N automatically? Yeah - it's certainly not a simple sed script like my previous transformations.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Also, this would have to be done for inalloca and sret as well - sret only<br>
needs it when targeting SPARC, although still generally useful in analysis.<br></blockquote><div><br></div><div>Reid, Dave - bringing this to your attention in case you've got any thoughts on inalloca/sret (was sret part of the Windows-y things, or just inalloca?), how they should be handled, if it should be different/the same as byval, etc?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">To sidestep some of the concerns and allow a smooth transition towards a<br>
byval that works with opaque pointers, I've come up with a new approach:<br>
<br>
Reuse dereferenceable(S) and align A for the size and alignment of byval.<br></blockquote><div><br></div><div>Side note: align should already be what's used for the alignment of byval. I think I remember seeing that there was one target hook and one target using that hook, that did some backend alignment fixes. If that is still present, we should fix/remove it so that the align attribute is the one source of truth for the alignment of a byval parameter. This should happen before any changes to byval.</div><div><br></div><div>The dereferenceable part is the novelty here and I really don't know how everyone feels about it. I don't much mind - I suppose I like it because it means optimizations can, maybe, hack on byval parameters more effectively without having to have byval-specific smarts necessarily. (also, because you've already done a bunch of the work)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">That is, a byval dereferenceable(S) align A argument is guaranteed to have<br>
S bytes available to read from, *and only S*, aligned to a multiple of A.<br>
Reading past that size is UB, as LLVM will not copy more than S bytes.<br>
<br>
An API can be provided to add the attribute alongside dereferenceable<br>
and align attributes, for a given Type* and DataLayout.<br>
<br>
A preliminary implementation (w/o sret) can be found at:<br>
<a href="https://github.com/eddyb/llvm/compare/2579466...65ac99b" rel="noreferrer" target="_blank">https://github.com/eddyb/llvm/compare/2579466...65ac99b</a><br>
<br>
To maintain compatibility with existing code,</blockquote><div><br></div><div>^ this isn't actually a goal/necessity, fwiw. So we might not need the complexity you're outlining below. Or if we do choose to take it, we may only want/need to keep it around for a relatively short period.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> dereferenceable and align<br>
attributes are automatically injected as soon as a non-default DataLayout<br>
is available. The "injection" mechanism could potentially be replaced with<br>
a pass, although it was easier to experiment with it being guaranteed.<br>
<br>
This works out pretty well in practice, as analysis already understands<br>
dereferenceable and can make decisions based on it.<br>
<br>
The verifier checks that for byval & friends, dereferenceable(S) and<br>
align A are present (clang always adds align, but not all tests have it)<br></blockquote><div><br></div><div>Do all tests need it? Align is just for non-default alignment (> align(1)) so it may be OK for the tests to omit it & it's probably not right to /require/ an align attribute on a byval parameter.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
and that S is the exact size of the pointee type (while we still know that).<br>
<br>
That last bit is very important, because it allows a script to do the following:<br>
<br>
1. Find all byval arguments in tests that are missing dereferenceable, e.g.<br>
... i32* byval align 4 ...<br>
.... {i8, i64}* byval ...<br>
2. Add a bogus dereferenceable(unique ID) to each of them, i.e.<br>
... i32* byval dereferenceable(123400001) align 4 ...<br>
.... {i8, i16}* byval dereferenceable(123400002) ...<br>
3. Run the tests and record the errors, which may look like:<br>
<br>
Attribute 'byval' expects 'dereferenceable(4)' for type i32*,<br>
found 'dereferenceable(123400001)'<br>
<br>
Attribute 'byval' expects 'dereferenceable(16) align 8' for type {i8, i64}*,<br>
found 'dereferenceable(123400002)'<br>
<br>
4. Use the verifier error messages to replace the bogus attributes<br>
with the proper ones, which include align A when it is missing:<br>
... i32* byval dereferenceable(4) align 4 ...<br>
.... {i8, i16}* byval dereferenceable(16) align 8 ...<br>
<br>
For what is worth, the same scheme would also work for byval(N), and<br>
would be entirely unnecessary for byval(T).<br>
<br>
I would love to know your thoughts on this, and more specifically:<br>
Which of the 3 (byval(T), byval(N) and byval + dereferenceable + align)<br>
do you think would provide the easiest transition path for front-ends?<br>
<br>
Thank you,<br>
- eddyb<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div><br></div></div>