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

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>

Hi Hal,

On Thu, Jan 5, 2017 at 11:10 AM, Hal Finkel via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> It is still only a function of its arguments, so it can be CSE'd.
That's a good example -- we can CSE it without worrying about the
memory state flowing in.

In fact, if we have:

*a = 10;
call @readnone_may_unwind()
*a = 20;
call @readnone_may_unwind()
*a = 30;

then we can first transform this to:

*a = 10;
call @readnone_may_unwind()
*a = 20;
call @readnone_may_unwind()  nounwind // only on this call (since we
returned normally)
*a = 30;

and then DSE:

*a = 10;
call @readnone_may_unwind()
// *a = 20;
call @readnone_may_unwind()  nounwind // only on this call
*a = 30;

> 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.
That's not specific to readnone though, right?  Even if the function
was readonly-mayunwind, the optimization would be legal.
>
>  -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
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

On 01/05/2017 01:20 PM, Sanjoy Das wrote:
> Hi Hal,
>
> On Thu, Jan 5, 2017 at 11:10 AM, Hal Finkel via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> It is still only a function of its arguments, so it can be CSE'd.
> That's a good example -- we can CSE it without worrying about the
> memory state flowing in.
>
> In fact, if we have:
>
> *a = 10;
> call @readnone_may_unwind()
> *a = 20;
> call @readnone_may_unwind()
> *a = 30;
>
> then we can first transform this to:
>
> *a = 10;
> call @readnone_may_unwind()
> *a = 20;
> call @readnone_may_unwind()  nounwind // only on this call (since we
> returned normally)
> *a = 30;
>
> and then DSE:
>
> *a = 10;
> call @readnone_may_unwind()
> // *a = 20;
> call @readnone_may_unwind()  nounwind // only on this call
> *a = 30;
>
>
>> 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.
> That's not specific to readnone though, right?  Even if the function
> was readonly-mayunwind, the optimization would be legal.
Yes, unless the readonly-mayunwind functions takes the memory as a 
parameter, in which case the latter sentence does not apply.

  -Hal
>
>>   -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
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory