llvm dev - Jan 2017 - RFC: Allow readnone and readonly functions to throw exceptions

On 01/05/2017 12:17 PM, Reid Kleckner wrote:
> On Thu, Jan 5, 2017 at 9:19 AM, Hal Finkel via llvm-dev 
> <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
>
>
>     On 01/05/2017 10:55 AM, Sanjoy Das wrote:
>
>         Hi Hal,
>
>         On Thu, Jan 5, 2017 at 6:12 AM, Hal Finkel <hfinkel at anl.gov
>         <mailto:hfinkel at anl.gov>> wrote:
>
>             On 01/04/2017 10:35 PM, Sanjoy Das via llvm-dev wrote:
>
>                 I just realized that there's an annoying corner case
>                 to this scheme --
>                 I can't DSE stores across readnone maythrow function
>                 calls because the
>                 exception handler could read memory. That is, in:
>
>                 try {
>                     *a = 10;
>                     call void @readnone_mayunwind_fn();
>                     *a = 20;
>                 } catch (...) {
>                     assert(*a == 10);
>                 }
>
>                 I can't DSE the `*a = 10` store.
>
>                 As far as I can tell, the most restrictive memory
>                 attribute for a
>                 potentially throwing function is readonly. "readnone
>                 may-unwind" does
>                 not make sense.
>
>
>             Why not? I've not followed this thread in detail, but it
>             seems like you're
>             discussing allowing the modeling of EH schemes that don't
>             access accessible
>             memory. In that case, a may-unwind readnone function is
>             just one that makes
>             its decision about if/what to throw based only on its
>             arguments.
>
>         If the call to @readnone_mayunwind_fn throws and I've
DSE'ed
>         the "*a >         10" store, the exception handler
will fail the *a == 10 assert
>         (assume
>         *a is not 10 to begin with).  The function call itself is
>         readnone,
>         but its exceptional continuation may read any part of the heap.
>
>         This isn't a big deal, but it means that "readnone
may-unwind"
>         will
>         effectively have to be treated as "readonly may-unwind"
-- I
>         don't see
>         any optimization that would be applicable to one and not the
>         other.
>         Maybe we should just move ahead with that (that readnone
>         may-unwind is
>         allowed, but if you want readnone-like optimizations then you
>         need to
>         also mark it as nounwind)?
>
>
>     Yes, I think that makes sense. The attribute only applies to the
>     function anyway, so what exception handlers might do (which is
>     assumed to be reading/writing any memory that might be available
>     to them) must be reasoned about separately.
>
>
> I don't think we need or want to do that. The way I see it, readonly 
> implies that the exception handler cannot write memory readable by 
> LLVM. Similarly, readnone should imply that the exception handler does 
> not read memory written by LLVM. Basically, any function that may 
> unwind but also has these attributes asserts that the exception 
> handler is operating outside of memory modeled by LLVM.
I don't understand why that's desirable, and I think it would severely 
limit our ability to infer these attributes for functions that unwind. 
You'd need to prove things -- likely unknowable things -- about the 
exception handlers in place around every call site of a function in 
order to mark it readonly, readnone, etc. We'd have the same problem 
with the attribute parameters. I'm fairly certain we do need and want to 
separate these concerns. This way we can apply callsite specific 
reasoning to the potential effects of exception handlers separate from 
what the function itself might do.

  -Hal
>
> I don't think we'll do DSE in your example because the store
isn't
> dead, it's visible along the invoke's unwind edge, and we don't
need
> to change the semantics of readnone to see that.
-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170105/be83f732/attachment.html>

> On Jan 5, 2017, at 10:39 AM, Hal Finkel via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> 
> On 01/05/2017 12:17 PM, Reid Kleckner wrote:
>> On Thu, Jan 5, 2017 at 9:19 AM, Hal Finkel via llvm-dev <llvm-dev at
lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>> 
>> On 01/05/2017 10:55 AM, Sanjoy Das wrote:
>> Hi Hal,
>> 
>> On Thu, Jan 5, 2017 at 6:12 AM, Hal Finkel <hfinkel at anl.gov
<mailto:hfinkel at anl.gov>> wrote:
>> On 01/04/2017 10:35 PM, Sanjoy Das via llvm-dev wrote:
>> I just realized that there's an annoying corner case to this scheme
--
>> I can't DSE stores across readnone maythrow function calls because
the
>> exception handler could read memory. That is, in:
>> 
>> try {
>>     *a = 10;
>>     call void @readnone_mayunwind_fn();
>>     *a = 20;
>> } catch (...) {
>>     assert(*a == 10);
>> }
>> 
>> I can't DSE the `*a = 10` store.
>> 
>> As far as I can tell, the most restrictive memory attribute for a
>> potentially throwing function is readonly.  "readnone
may-unwind" does
>> not make sense.
>> 
>> Why not? I've not followed this thread in detail, but it seems like
you're
>> discussing allowing the modeling of EH schemes that don't access
accessible
>> memory. In that case, a may-unwind readnone function is just one that
makes
>> its decision about if/what to throw based only on its arguments.
>> If the call to @readnone_mayunwind_fn throws and I've DSE'ed
the "*a >> 10" store, the exception handler will fail the *a ==
10 assert (assume
>> *a is not 10 to begin with).  The function call itself is readnone,
>> but its exceptional continuation may read any part of the heap.
>> 
>> This isn't a big deal, but it means that "readnone
may-unwind" will
>> effectively have to be treated as "readonly may-unwind" -- I
don't see
>> any optimization that would be applicable to one and not the other.
>> Maybe we should just move ahead with that (that readnone may-unwind is
>> allowed, but if you want readnone-like optimizations then you need to
>> also mark it as nounwind)?
>> 
>> Yes, I think that makes sense. The attribute only applies to the
function anyway, so what exception handlers might do (which is assumed to be
reading/writing any memory that might be available to them) must be reasoned
about separately.
>> 
>> I don't think we need or want to do that. The way I see it,
readonly implies that the exception handler cannot write memory readable by
LLVM. Similarly, readnone should imply that the exception handler does not read
memory written by LLVM. Basically, any function that may unwind but also has
these attributes asserts that the exception handler is operating outside of
memory modeled by LLVM.
> 
> I don't understand why that's desirable, and I think it would
severely limit our ability to infer these attributes for functions that unwind.
You'd need to prove things -- likely unknowable things -- about the
exception handlers in place around every call site of a function in order to
mark it readonly, readnone, etc. We'd have the same problem with the
attribute parameters. I'm fairly certain we do need and want to separate
these concerns. This way we can apply callsite specific reasoning to the
potential effects of exception handlers separate from what the function itself
might do.
What useful things would you be able to deduce from an “unwind readnone”
function under these conditions?

>> I don't think we'll do DSE in your example because the store
isn't dead, it's visible along the invoke's unwind edge, and we
don't need to change the semantics of readnone to see that.
I’ve been wondering the same thing on Sanjoy’s example.

— 
Mehdi

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170105/0597cf69/attachment.html>

Hi Mehdi,

On Thu, Jan 5, 2017 at 10:45 AM, Mehdi Amini via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
[snip]
> I don't understand why that's desirable, and I think it would
severely limit
> our ability to infer these attributes for functions that unwind. You'd
need
> to prove things -- likely unknowable things -- about the exception handlers
> in place around every call site of a function in order to mark it readonly,
> readnone, etc. We'd have the same problem with the attribute
parameters. I'm
> fairly certain we do need and want to separate these concerns. This way we
> can apply callsite specific reasoning to the potential effects of exception
> handlers separate from what the function itself might do.
>
>
> What useful things would you be able to deduce from an “unwind readnone”
> function under these conditions?
In theory you could do this transform:

void f(int*a) {
  *a = 20;
  call @mayunwind_readnone();
  *a = 30;
}

=>

void f(int*a) {
  // *a = 20;
  invoke @mayunwind_readnone() to %normal unwind %unwind

normal:
  *a = 30;
  ret void

unwind:
  %exc = landingpad
  *a = 20;
  resume %exc
}

(That is, PRE the store around the implicit edge by making it explicit.)

Generally though I don't see us doing this except under exceptional
(no pun intended!) circumstances.

-- Sanjoy


>
>
> I don't think we'll do DSE in your example because the store
isn't dead,
> it's visible along the invoke's unwind edge, and we don't need
to change the
> semantics of readnone to see that.
>
>
> I’ve been wondering the same thing on Sanjoy’s example.
>
> —
> Mehdi
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

On 01/05/2017 12:45 PM, Mehdi Amini wrote:
>
>> On Jan 5, 2017, at 10:39 AM, Hal Finkel via llvm-dev 
>> <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
>>
>>
>> On 01/05/2017 12:17 PM, Reid Kleckner wrote:
>>> On Thu, Jan 5, 2017 at 9:19 AM, Hal Finkel via llvm-dev 
>>> <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
>>>
>>>
>>>     On 01/05/2017 10:55 AM, Sanjoy Das wrote:
>>>
>>>         Hi Hal,
>>>
>>>         On Thu, Jan 5, 2017 at 6:12 AM, Hal Finkel <hfinkel at
anl.gov
>>>         <mailto:hfinkel at anl.gov>> wrote:
>>>
>>>             On 01/04/2017 10:35 PM, Sanjoy Das via llvm-dev wrote:
>>>
>>>                 I just realized that there's an annoying corner
case
>>>                 to this scheme --
>>>                 I can't DSE stores across readnone maythrow
function
>>>                 calls because the
>>>                 exception handler could read memory. That is, in:
>>>
>>>                 try {
>>>                     *a = 10;
>>>                     call void @readnone_mayunwind_fn();
>>>                     *a = 20;
>>>                 } catch (...) {
>>>                     assert(*a == 10);
>>>                 }
>>>
>>>                 I can't DSE the `*a = 10` store.
>>>
>>>                 As far as I can tell, the most restrictive memory
>>>                 attribute for a
>>>                 potentially throwing function is readonly. 
>>>                 "readnone may-unwind" does
>>>                 not make sense.
>>>
>>>
>>>             Why not? I've not followed this thread in detail,
but it
>>>             seems like you're
>>>             discussing allowing the modeling of EH schemes that
>>>             don't access accessible
>>>             memory. In that case, a may-unwind readnone function is
>>>             just one that makes
>>>             its decision about if/what to throw based only on its
>>>             arguments.
>>>
>>>         If the call to @readnone_mayunwind_fn throws and I've
DSE'ed
>>>         the "*a >>>         10" store, the
exception handler will fail the *a == 10
>>>         assert (assume
>>>         *a is not 10 to begin with).  The function call itself is
>>>         readnone,
>>>         but its exceptional continuation may read any part of the
heap.
>>>
>>>         This isn't a big deal, but it means that "readnone
>>>         may-unwind" will
>>>         effectively have to be treated as "readonly
may-unwind" -- I
>>>         don't see
>>>         any optimization that would be applicable to one and not
the
>>>         other.
>>>         Maybe we should just move ahead with that (that readnone
>>>         may-unwind is
>>>         allowed, but if you want readnone-like optimizations then
>>>         you need to
>>>         also mark it as nounwind)?
>>>
>>>
>>>     Yes, I think that makes sense. The attribute only applies to
the
>>>     function anyway, so what exception handlers might do (which is
>>>     assumed to be reading/writing any memory that might be
available
>>>     to them) must be reasoned about separately.
>>>
>>>
>>> I don't think we need or want to do that. The way I see it,
readonly
>>> implies that the exception handler cannot write memory readable by 
>>> LLVM. Similarly, readnone should imply that the exception handler 
>>> does not read memory written by LLVM. Basically, any function that 
>>> may unwind but also has these attributes asserts that the exception
>>> handler is operating outside of memory modeled by LLVM.
>>
>> I don't understand why that's desirable, and I think it would 
>> severely limit our ability to infer these attributes for functions 
>> that unwind. You'd need to prove things -- likely unknowable things
>> -- about the exception handlers in place around every call site of a 
>> function in order to mark it readonly, readnone, etc. We'd have the
>> same problem with the attribute parameters. I'm fairly certain we
do
>> need and want to separate these concerns. This way we can apply 
>> callsite specific reasoning to the potential effects of exception 
>> handlers separate from what the function itself might do.
>
> What useful things would you be able to deduce from an “unwind 
> readnone” function under these conditions?
It is still only a function of its arguments, so it can be CSE'd.

Also, if I have this:

  *a = 10;
  b = a_readnone_unwind_func();
  *a = 10;

I can still conclude that this last store is redundant and can be 
removed. I know that the readnone function does not touch it, and if it 
unwinds, than the later store is dead. If I know that &*a has not 
escaped to where an exception handler might access it, then I know that 
the first store than be removed.

  -Hal
>
>
>>> I don't think we'll do DSE in your example because the
store isn't
>>> dead, it's visible along the invoke's unwind edge, and we
don't need
>>> to change the semantics of readnone to see that.
>
> I’ve been wondering the same thing on Sanjoy’s example.
>
> —
> Mehdi
>
-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170105/dcb16899/attachment.html>

On Thu, Jan 5, 2017 at 10:39 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> I don't understand why that's desirable, and I think it would
severely
> limit our ability to infer these attributes for functions that unwind.
> You'd need to prove things -- likely unknowable things -- about the
> exception handlers in place around every call site of a function in order
> to mark it readonly, readnone, etc. We'd have the same problem with the
> attribute parameters. I'm fairly certain we do need and want to
separate
> these concerns. This way we can apply callsite specific reasoning to the
> potential effects of exception handlers separate from what the function
> itself might do.
>
I'm really just trying to push exception handlers outside the LLVM model.
They shouldn't be able to look at LLVM values, so that we *can* reason
locally to infer readnone/readonly.

In practice, when could we infer readnone without inferring nounwind? The
only thing in LLVM that can throw is a call or intrinsic call to EH
machinery, and that instruction will need to be attributed with knowledge
of the exception handler. If the exception handler really doesn't write
memory that LLVM can read, then we can mark it readonly, and our normal
local inference will work.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170105/44c3baa5/attachment.html>

On 01/05/2017 03:10 PM, Reid Kleckner wrote:
> On Thu, Jan 5, 2017 at 10:39 AM, Hal Finkel <hfinkel at anl.gov 
> <mailto:hfinkel at anl.gov>> wrote:
>
>     I don't understand why that's desirable, and I think it would
>     severely limit our ability to infer these attributes for functions
>     that unwind. You'd need to prove things -- likely unknowable
>     things -- about the exception handlers in place around every call
>     site of a function in order to mark it readonly, readnone, etc.
>     We'd have the same problem with the attribute parameters. I'm
>     fairly certain we do need and want to separate these concerns.
>     This way we can apply callsite specific reasoning to the potential
>     effects of exception handlers separate from what the function
>     itself might do.
>
> I'm really just trying to push exception handlers outside the LLVM 
> model. They shouldn't be able to look at LLVM values, so that we *can* 
> reason locally to infer readnone/readonly.
Maybe we're not on the same page. When I say exception handler, I mean 
any code the unwinding in-turn calls - i.e. anything in a catch block.
>
> In practice, when could we infer readnone without inferring nounwind? 
> The only thing in LLVM that can throw is a call or intrinsic call to 
> EH machinery, and that instruction will need to be attributed with 
> knowledge of the exception handler. If the exception handler really 
> doesn't write memory that LLVM can read, then we can mark it readonly, 
> and our normal local inference will work.
What do you mean? Any call, except for those tagged with nounwind, can 
throw. They don't need to be 'invoked'. I don't like this aspect
of
LLVM's IR, but that's another story.

  -Hal

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170105/6de1464a/attachment.html>

On Thu, Jan 5, 2017 at 1:10 PM, Reid Kleckner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On Thu, Jan 5, 2017 at 10:39 AM, Hal Finkel <hfinkel at anl.gov>
wrote:
>
>> I don't understand why that's desirable, and I think it would
severely
>> limit our ability to infer these attributes for functions that unwind.
>> You'd need to prove things -- likely unknowable things -- about the
>> exception handlers in place around every call site of a function in
order
>> to mark it readonly, readnone, etc. We'd have the same problem with
the
>> attribute parameters. I'm fairly certain we do need and want to
separate
>> these concerns. This way we can apply callsite specific reasoning to
the
>> potential effects of exception handlers separate from what the function
>> itself might do.
>>
> I'm really just trying to push exception handlers outside the LLVM
model.
> They shouldn't be able to look at LLVM values, so that we *can* reason
> locally to infer readnone/readonly.
>
So the WinEH C++ exception handler actually *does* write to LLVM values: it
calls copy constructors and destructors for catch objects. However, we do
not model it as such in LLVM. We _attribute_ the memory modification for
the copy-constructor to the CatchPad and the memory modification for the
destructor to the CatchRet.

I think it is reasonable that all memory operations performed by a
personality routine "loudly" describe their ability to load/store to
memory
using some instruction instead of implicitly by virtue of the function body
having been marked with a personality.

>
> In practice, when could we infer readnone without inferring nounwind? The
> only thing in LLVM that can throw is a call or intrinsic call to EH
> machinery, and that instruction will need to be attributed with knowledge
> of the exception handler. If the exception handler really doesn't write
> memory that LLVM can read, then we can mark it readonly, and our normal
> local inference will work.
>
> _______________________________________________
> 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/20170105/88f6e253/attachment.html>