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

[Sanjoy Das via llvm-dev]

On Thu, Jan 5, 2017 at 10:17 AM, Reid Kleckner <rnk at google.com> wrote:
> On Thu, Jan 5, 2017 at 9:19 AM, Hal Finkel via llvm-dev
> <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> 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.

What Hal said, basically -- I'd rather not lose the ability to locally
reason about these attributes.

> 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 did not give a full example for brevity, but I was really going for:

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

Today EarlyCSE will DSE the first store to `a`, even though an
exception handler further up the stack could be reading from `a`.  If
we allow "readnone may_unwind" functions then we'll have to be more
careful (than we are today) around implicit out-edges like this.

-- Sanjoy