<div dir="ltr">I've always been in the "readnone implies nounwind" camp, but I've changed my mind, and I'm in favor of this RFC now.<div><br></div><div>I think in the past I was hung up on the idea that every language typically has some "exception object" that is stored in memory, and the user code interrogates it by loading, so we need to model stores when throwing an exception. Right now I'm imagining an EH personality that uses i32 error codes as its exceptional return value, which would look like this:</div><div><br></div><div>  invoke void @f() to label %next unwind label %lpad ; could be readnone</div><div>...</div><div>lpad:</div><div>  %vals = i32 landingpad ... ; suppose there is no selector value in this personality</div><div><br></div><div>define void @f() readnone {</div><div>  resume i32 42  ; imagine that resume could initiate exceptional control flow like the old 'unwind' instruction<br>}</div><div><br></div><div>With a personality like this, there are no llvm-visible memory operations looking at the exception, so it makes sense for @f to be readnone.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 2, 2017 at 10:18 PM, Sanjoy Das 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">LLVM today does not clearly specify if a function specified to not<br>
write to memory (i.e. readonly or readnone) is allowed to throw<br>
exceptions.<br>
<br>
LangRef is ambiguous on this issue.  The normative statement is<br>
"[readnone/readonly functions] cannot unwind exceptions by calling the<br>
C++ exception throwing methods" which does not decide an answer for<br>
non C++ languages.  It used to say (h/t Daniel Berlin): "This means<br>
that it cannot unwind exceptions by calling the C++ exception throwing<br>
methods, but could use the unwind instruction.", but that bit of<br>
documentation died with the unwind instruction.<br>
<br>
I'd like to separate unwindability from memory effects, and officially<br>
change our stance to be "readonly / readnone functions are allowed to<br>
throw exceptions".<br>
<br>
Here are two supporting reasons:<br>
<br>
# `resume` is already modeled as readnone<br>
<br>
The easiest way to verify this is via FunctionAttrs; it infers the<br>
following function as readnone:<br>
<br>
define void @f() personality i8 42 {<br>
  resume i32 0<br>
}<br>
<br>
<br>
Modeling `resume` as `readnone` is defensible -- it is a control flow<br>
transfer instruction, not so different from `ret`.  Moreover, it<br>
_cannot_ be modeled as having observable side effects or writes to<br>
memory (`resume` cannot send packets over the network or write to a<br>
global) because otherwise we'd be unable to inline @f into @g below:<br>
<br>
define void @f(i32 %x) personality i32 3 {<br>
  resume i32 %x<br>
}<br>
<br>
define i32 @g(i32 %x) personality i32 3 {<br>
  invoke void @f(i32 %x) to label %normal unwind label %unwind<br>
<br>
normal:<br>
  ret i32 0<br>
<br>
unwind:<br>
  %t = landingpad i32 cleanup<br>
  ret i32 %t<br>
}<br>
<br>
since it gets rid of a `resume` and thus a side effect (by<br>
assumption).<br>
<br>
<br>
# We're probably already there (but we need an audit)<br>
<br>
All said and done, the situation is not as "loosey goosey" as I made<br>
it sound like.  mayHaveSideEffects() is defined as "mayWriteToMemory()<br>
|| mayThrow()"; and this shows in e.g. EarlyCSE which will refuse to<br>
DCE the call to @f in @g<br>
<br>
declare void @f() readnone<br>
<br>
define void @g() {<br>
  call void @f()<br>
  ret void<br>
}<br>
<br>
unless @f is also marked nounwind.<br>
<br>
I've already fixed the one other instance I was aware of in<br>
<a href="https://reviews.llvm.org/rL290794" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>rL290794</a> (but I will revert that patch if we<br>
decide against this RFC).<br>
<br>
We won't lose any expressive power either -- if there are situations<br>
where we have important optimizations firing under the "readonly<br>
implies nounwind" assumption, we can either<br>
<br>
 - Teach FunctionAttrs to infer nounwind for readonly functions with<br>
   C++ unwind personalities.<br>
<br>
 - For external declarations generated by the compiler (say from the<br>
   standard library): if the functions are actually nounwind, mark<br>
   them as nounwind; and not rely on LLVM inferring nounwind from<br>
   readonly.<br>
<br>
<br>
My (unrealistic?) hope is that this would mostly be a specification<br>
change and not involve a lot of code fixes.<br>
<br>
The change is also trivially upgrade-safe for older bitcode -- calls<br>
to readonly / readnone functions that do not throw _may_ get optimized<br>
less, but that should not be a correctness problem.<br>
<br>
What do you think?<br>
<br>
-- Sanjoy<br>
______________________________<wbr>_________________<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/<wbr>mailman/listinfo/llvm-dev</a><br>
</blockquote></div><br></div>