<div dir="ltr">+1 to plan<div><br></div><div>It would be good to include inference logic to promote to full `dereferenceable` when possible so that transforms which need that anyways can just use that.</div><div><br></div><div>And/or we should ensure the APIs used always query both so that transform authors don't have to remember to do so...</div><div><br></div><div>I still somewhat wish we could do with a single `dereferenceable`.</div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 11, 2018 at 5:21 PM Hal Finkel via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<p>Thanks, Richard.</p>
<p>Based on the feedback from this thread, I'll move forward with
the patches for nofree, nosync, adding a new corresponding
dereferenceable attribute (my suggestion is to name this
dereferenceable_on_entry; suggestions welcome), and updating Clang
is emit this new attribute instead of the current one.<br>
</p></div><div text="#000000" bgcolor="#FFFFFF">
-Hal</div><div text="#000000" bgcolor="#FFFFFF"><br>
<br>
<div class="m_-277137984432361038moz-cite-prefix">On 07/11/2018 06:43 PM, Richard Smith
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr">On Wed, 11 Jul 2018 at 16:13, Hal Finkel via
llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">[+Richard]<br>
<br>
<br>
On 07/11/2018 08:29 AM, Sanjoy Das wrote:<br>
> I'm not sure if nosynch is sufficient. What if we had:<br>
><br>
> void f(int& x) {<br>
> if (false) {<br>
> int r0 = x;<br>
> }<br>
> }<br>
><br>
> // other thread<br>
> free(<pointer to x>);<br>
><br>
> The source program is race free, but LLVM may speculate
the read from<br>
> x (seeing that it is dereferenceable) creating a race.<br>
<br>
Interestingly, I'm not sure. I trust that Richard can answer
this<br>
question. :-)<br>
<br>
So, if we had:<br>
<br>
int y = ...;<br>
...<br>
f(y);<br>
<br>
then I think that Clang's use of dereferenceable is almost
certainly<br>
okay (because the standard explicitly says, 9.2.3.2p5, "A
reference<br>
shall be initialized to refer to a valid object or<br>
function."). Because the reference must have been valid when
f(y) began<br>
executing, unless it synchronizes somehow with the other
thread, any<br>
asynchronous deletion of y must be a race.<br>
<br>
On the other hand, if we have:<br>
<br>
int &y = ...;<br>
...<br>
f(y);<br>
<br>
do we know that, when f(y) begins executing, the reference
points to a<br>
valid object? My reading of 9.3.3p2, which says, "Argument
passing<br>
(7.6.1.2) and<br>
function value return (8.6.3) are initializations.",
combined with the<br>
statement above, implies that, perhaps surprisingly, the
same holds<br>
here. When the argument to f is initialized, it must refer
to a valid<br>
object (even if the initializer is another reference).<br>
<br>
Richard, what do you think?<br>
</blockquote>
<div><br>
</div>
<div>First, see also <a href="http://wg21.link/cwg453" target="_blank">core issue 453</a>, under the guise
of which we're fixing the wording in [<span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">dcl.ref](9.2.3.2)p5
from</span></div>
<div><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br>
</span></div>
<div><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">
"<span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">A
reference shall be initialized to refer to a valid
object or function."</span></span></div>
<div><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br>
</span></span></div>
<div><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">to
something like</span></span></div>
<div><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br>
</span></span></div>
<div><span style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">
"</span></span>If an lvalue to which a reference is
directly bound designates neither an existing object or
function of an appropriate type (11.6.3 [dcl.init.ref]), nor
a region of storage of suitable size and alignment to
contain an object of the reference's type (4.5
[intro.object], 6.8 [basic.life], 6.9 [basic.types]), the
behavior is undefined."</div>
<div><br>
</div>
<div>My take is that, if the end of the duration of the region
of storage is unsequenced with respect to the binding of the
reference, then behavior is undefined. Generally when we
refer to a thing happening while some condition is true, we
mean that the execution point when the condition became true
is sequenced before the thing happening, and the execution
point where it becomes not true again is sequenced after.</div>
<div><br>
</div>
<div>So the behavior of that program is undefined regardless
of whether 'f' actually loads through 'x'.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Thanks again,<br>
Hal<br>
<br>
P.S. If I'm right, then I might be happy, but it's also
somewhat scary<br>
(although we've been doing this optimization for multiple
releases and I<br>
don't think we have a bug along these lines), and I'd at
least smell the<br>
need for a sanitizer.<br>
<br>
><br>
> -- Sanjoy<br>
> On Tue, Jul 10, 2018 at 7:01 PM Hal Finkel via llvm-dev<br>
> <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>
wrote:<br>
>> Hi, everyone,<br>
>><br>
>> I'd like to propose adding a nofree function
attribute to indicate that<br>
>> a function does not, directly or indirectly, call a
memory-deallocation<br>
>> function (e.g., free, C++'s operator delete).
Clang/LLVM can currently<br>
>> misoptimize functions that:<br>
>><br>
>> 1. Have a reference argument.<br>
>><br>
>> 2. Free the memory backing the object to which the
reference is bound<br>
>> during the function's execution.<br>
>><br>
>> Because we tag, in Clang, all reference arguments
using the<br>
>> dereferenceable attribute, LLVM assumes that the
pointer is<br>
>> unconditionally dereferenceable throughout the
course of the entire<br>
>> function. This isn't true, however, if the memory
is freed during the<br>
>> execution of the function. For more information,
please see the<br>
>> discussion in <a href="https://reviews.llvm.org/D48239" rel="noreferrer" target="_blank">https://reviews.llvm.org/D48239</a>.<br>
>><br>
>> To solve this problem, we need to give LLVM more
information in order to<br>
>> help it determine when a pointer, which is
dereferenceable when the<br>
>> functions begins to execute, will still be
dereferenceable later on in<br>
>> the function's execution. This nofree attribute can
be part of that<br>
>> solution. If we know that free (and friends) are
not called by the<br>
>> function (nor by any function called by the
function, and so on), then<br>
>> we know that pointers that started out
dereferenceable will stay that<br>
>> way (except as explained below).<br>
>><br>
>> I'm initially proposing this to be only a function
attribute, although<br>
>> one could easily imagine a parameter attribute as
well (that indicates<br>
>> that a particular pointer argument is not freed by
the function). This<br>
>> might be useful, but for the use case of helping
dereferenceable, it<br>
>> would be subtle to use, unless the parameter was
also marked as noalias,<br>
>> because you'd need to know that the parameter was
not also aliased with<br>
>> another argument (or had not been captured).
Another analysis would need<br>
>> to provide this kind of information.<br>
>><br>
>> Also, just because a function does not, directly or
indirectly, call<br>
>> free does not mean that it cannot cause memory to
be deallocated. The<br>
>> function might communicate (synchronize) with
another thread causing<br>
>> that other thread to delete the memory. For this
reason, to use<br>
>> dereferenceable as we currently do, we also need to
know that the<br>
>> function does not synchronize with any other
threads. To solve this<br>
>> problem, like nofree, I propose to add a nosynch
attribute (to indicate<br>
>> that a function does not use (non-relaxed) atomics
or otherwise<br>
>> synchronize with any other threads (e.g., perform
I/O or, as a practical<br>
>> matter, use volatile accesses).<br>
>><br>
>> I've posted a patch for the nofree attribute<br>
>> (<a href="https://reviews.llvm.org/D49165" rel="noreferrer" target="_blank">https://reviews.llvm.org/D49165</a>).
nosynch's implementation would be<br>
>> very similar (except instead of looking for calls
to free, it would look<br>
>> for uses of non-relaxed atomics, volatile ops, and
known functions that<br>
>> are not I/O functions).<br>
>><br>
>> With both of these attributes (nofree and nosynch),
a function argument<br>
>> with the dereferenceable attribute will be known to
be dereferenceable<br>
>> throughout the execution of the attributed
function. We can update<br>
>> isDereferenceableAndAlignedPointer to include these
additional checks on<br>
>> the current function.<br>
>><br>
>> One more choice we have: We can, as I proposed
above, essentially weaken<br>
>> the current semantics of dereferenceable to not
exclude<br>
>> mid-function-execution deallocation. We can also
add a second attribute<br>
>> with the current, stronger, semantics. We can keep
the current attribute<br>
>> as-is, and add a second attribute with the weaker
semantics (and switch<br>
>> Clang to use that).<br>
>><br>
>> Please let me know what you think.<br>
>><br>
>> Thanks again,<br>
>><br>
>> Hal<br>
>><br>
>> --<br>
>> Hal Finkel<br>
>> Lead, Compiler Technology and Programming Languages<br>
>> Leadership Computing Facility<br>
>> Argonne National Laboratory<br>
>><br>
>> _______________________________________________<br>
>> LLVM Developers mailing list<br>
>> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">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>
<br>
-- <br>
Hal Finkel<br>
Lead, Compiler Technology and Programming Languages<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">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>
</div>
</blockquote>
<br>
<pre class="m_-277137984432361038moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">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>