[llvm-dev] RFC: Allow readnone and readonly functions to throw exceptions

[Sanjoy Das via llvm-dev]

LLVM today does not clearly specify if a function specified to not
write to memory (i.e. readonly or readnone) is allowed to throw
exceptions.

LangRef is ambiguous on this issue.  The normative statement is
"[readnone/readonly functions] cannot unwind exceptions by calling the
C++ exception throwing methods" which does not decide an answer for
non C++ languages.  It used to say (h/t Daniel Berlin): "This means
that it cannot unwind exceptions by calling the C++ exception throwing
methods, but could use the unwind instruction.", but that bit of
documentation died with the unwind instruction.

I'd like to separate unwindability from memory effects, and officially
change our stance to be "readonly / readnone functions are allowed to
throw exceptions".

Here are two supporting reasons:

# `resume` is already modeled as readnone

The easiest way to verify this is via FunctionAttrs; it infers the
following function as readnone:

define void @f() personality i8 42 {
  resume i32 0
}


Modeling `resume` as `readnone` is defensible -- it is a control flow
transfer instruction, not so different from `ret`.  Moreover, it
_cannot_ be modeled as having observable side effects or writes to
memory (`resume` cannot send packets over the network or write to a
global) because otherwise we'd be unable to inline @f into @g below:

define void @f(i32 %x) personality i32 3 {
  resume i32 %x
}

define i32 @g(i32 %x) personality i32 3 {
  invoke void @f(i32 %x) to label %normal unwind label %unwind

normal:
  ret i32 0

unwind:
  %t = landingpad i32 cleanup
  ret i32 %t
}

since it gets rid of a `resume` and thus a side effect (by
assumption).


# We're probably already there (but we need an audit)

All said and done, the situation is not as "loosey goosey" as I made
it sound like.  mayHaveSideEffects() is defined as "mayWriteToMemory()
|| mayThrow()"; and this shows in e.g. EarlyCSE which will refuse to
DCE the call to @f in @g

declare void @f() readnone

define void @g() {
  call void @f()
  ret void
}

unless @f is also marked nounwind.

I've already fixed the one other instance I was aware of in
https://reviews.llvm.org/rL290794 (but I will revert that patch if we
decide against this RFC).

We won't lose any expressive power either -- if there are situations
where we have important optimizations firing under the "readonly
implies nounwind" assumption, we can either

 - Teach FunctionAttrs to infer nounwind for readonly functions with
   C++ unwind personalities.

 - For external declarations generated by the compiler (say from the
   standard library): if the functions are actually nounwind, mark
   them as nounwind; and not rely on LLVM inferring nounwind from
   readonly.


My (unrealistic?) hope is that this would mostly be a specification
change and not involve a lot of code fixes.

The change is also trivially upgrade-safe for older bitcode -- calls
to readonly / readnone functions that do not throw _may_ get optimized
less, but that should not be a correctness problem.

What do you think?

-- Sanjoy