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

Tue Jan 3 10:19:23 PST 2017

On Tue, Jan 3, 2017 at 12:59 AM, Chandler Carruth via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> I like the direction of this RFC and agree with Michael's points about it.
>
> The "pure" and "const" history is definitely there, but I don't think it
> makes sense any more. I think narrow, precise, and well specified
> attributes are *much* better for LLVM's IR, especially as we diversify the
> set of frontends and language semantics we support.
>
> There will be plenty of code changes required, but I think the changes are
> tractable (these are relatively easy to audit for) and not risky. If Sanjoy
> has the cycles to run with this, fantastic.
>
> One thing we should make sure to do is update the langref to be *really
> clear* here. =] But I suspect Sanjoy is all over that.
>

This is pretty much what started this (and thank you very much to Sanjoy
for being willing to  to get this resolved :P)

The current text for readonly is "On a function, this attribute indicates
that the function does not ...  *modify any state (e.g. memory, control
registers, etc) visible to caller functions*. ... 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*."

Which, while, the lawyer part of me enjoys the amount of hair splitting
this allows, the other part of me is in the "what is this really trying to
mean" category :)

On Mon, Jan 2, 2017 at 11:49 PM Michael Kuperstein via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> This sounds right to me.
>
> IIUC, historically, readonly and readnone are meant to model the "pure"
> and "const" GCC attributes. These attributes make pretty strong guarantees:
>
> "[a pure] function can be subject to common subexpression elimination and
> loop optimization just as an arithmetic operator would be. These functions
> should be declared with the attribute pure [...] Interesting non-pure
> functions are functions with infinite loops or those depending on volatile
> memory or other system resource, that may change between two consecutive
> calls (such as feof in a multithreading environment)."
>
> In particular, pure/const imply termination - something that's not
> entirely clear w.r.t readonly. However, apparently, they don't imply
> nothrow. I've actually always thought they *do* imply it - and said so
> on-list :-) - but it looks like GCC itself doesn't interpret them that way.
> E.g. see John Regher's example here: https://t.co/REzy5m1tT3
> So there's at least one use-case for possibly throwing readonly/readnone.
>
> As a side note, I'm slightly less optimistic about the amount of required
> code fixes. One thing that comes to mind is that we need to make sure we
> mark all(?) the intrinsics currently marked readonly/argmemonly/readnone as
> nothrow. This should be mostly mechanical, I hope, but it's a decent amount
> of churn.
>
> Michael
>
>
>
>
> On 2 January 2017 at 22:18, Sanjoy Das via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> 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
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170103/dae53aec/attachment.html>