<div dir="ltr"><div>What about readnone?</div><div>The last time i asked, i got the answer that readnone implied nounwind.<br></div><div><br></div><div>Also, the langref currently says</div><div><br></div><div>readnone:<br><br></div><div>...</div><div>It does not write through any pointer arguments (including byval arguments) and never changes any state visible to callers. This means that it cannot unwind exceptions by calling the C++ exception throwing methods.</div><div><br></div><div>readonly:<br>...</div><div>A readonly function always returns the same value (or unwinds an exception identically) when called with the same set of arguments and global state. It cannot unwind an exception by calling the C++ exception throwing methods...</div><div><br></div><div><br></div><div>I'm not an expert in this area by any means, but if i was to read this, i would think it implied nounwind[1].</div><div>So if that's not what we mean, we should change it :)<br><br></div><div>Thus, IMHO this patch deserves a bit of discussion, with the  answer to whether they are nounwind being explicitly written down in the langref in a way dumb people like me can understand it. :)</div><div><br></div><div>--Dan</div><div><br></div><div>[1]   Note also the hilarious contradiction of readonly saying it unwinds an exception identically but then saying it can't unwind an exception :)</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Dec 31, 2016 at 2:25 PM, Sanjoy Das via Phabricator via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@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">sanjoy added a comment.<br>
<span class=""><br>
In <a href="https://reviews.llvm.org/D28147#632595" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D28147#632595</a>, @mkuper wrote:<br>
<br>
> So, the patch is now fairly small, but I'm having trouble with the testcases.<br>
>  In particular, I think what we have now is unsound, because isGuaranteedToTransferExecutio<wbr>nToSuccessor() returns true for argmemonly calls, which means that things that should only get promoted with my patch get promoted without.<br>
<br>
<br>
</span>That was an oversight, fixed in <a href="https://reviews.llvm.org/rL290794" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>rL290794</a><br>
<span class="im HOEnZb"><br>
> I'll get back to this on Tuesday. For now, I'll upload the code change, without fixing the tests, in case you want to take a look at the semantic change.<br>
<br>
<br>
<br>
<br>
</span><div class="HOEnZb"><div class="h5"><a href="https://reviews.llvm.org/D28147" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D28147</a><br>
<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>