llvm dev - Feb 2016 - RFC: Add guard intrinsics to LLVM

Hi Andy,

Responses inline:

On Mon, Feb 22, 2016 at 10:08 AM, Andrew Trick <atrick at apple.com>
wrote:
>> By memory barrier, do you mean things like fences?
> That’s right. Conservatively, I would not hoist at the LLVM level past
> opaque non-readonly calls or fences.
Just for curiosity:  why do you care about memory fences specifically?
> I've begun to think that may-unwind (or may fail guard) and readonly
> should be mutually exclusive. readonly refers to all system memory,
> not just LLVM-visible memory. To achieve the effect of
> "aliases-with-nothing-in-llvm" it's cleaner to use alias
analysis.
(I've put this on the bug as well:)

https://llvm.org/bugs/show_bug.cgi?id=18912#c3

```
(In reply to comment #2)
> The test case in this bug report is fixed by
> http://reviews.llvm.org/rL256728.
>
> I'm closing this because I don't have a test case and I no longer
think it
> makes much sense to mark may-unwind calls "readonly". An unwind
path always
> touches some memory. That fact that it doesn't alias with LLVM-visible
I'm not sure about this -- why does an unwind path *have* to write
some memory? Can't you implement unwind as "read the appropriate
exception handler RPC from a table, and return to that RPC"?
```
> memory access can be handled by alias analysis.
Btw, I think in this interpretation it is incorrect for -functionattrs
to infer readnone for @foo in (which it does today):

define void @foo({ i8*, i32 } %val) personality i8 8 {
  resume { i8*, i32 } %val
}
> I meant readonly can always be a property of the call site as opposed an
intrinsic
> property for more frontend control
Yes, I agree that if we decide that readonly semantics are fine, we
can always explicitly mark these as readonly, and not have to worry
about specifying the memory effects as part of the intrinsics
definition.  Part of the discussion here has devolved to is "readonly"
is correct.
> I was thinking that that "unwind" would be repurposed as you say.
But
> after responding to your points below, I think that could be
> misleading. It's probably cleaner to adhere to the rule that unwinding
> can only resume in a landing pad.
Ok.
> I understand your problem. I was ignoring an aspect of analyzing the
> unwind path and the fact the LLVM could make assumptions about where
> control will resume.
>
> mayBeOverriden makes sense. I would think that the symbol at the deopt
> call site needs to be marked mayBeOverriden.
Ok.  I'll probably split off an "introduce a `interposable` function
attribute" into an independent review (I think re-using a linkage type
for this will be somewhat of a hack).

Thanks!
-- Sanjoy

> On Feb 22, 2016, at 11:02 AM, Sanjoy Das <sanjoy at
playingwithpointers.com> wrote:
> 
> Hi Andy,
> 
> Responses inline:
> 
> On Mon, Feb 22, 2016 at 10:08 AM, Andrew Trick <atrick at apple.com>
wrote:
>>> By memory barrier, do you mean things like fences?
>> That’s right. Conservatively, I would not hoist at the LLVM level past
>> opaque non-readonly calls or fences.
> 
> Just for curiosity:  why do you care about memory fences specifically?
@trap_on would produce a side effect (crashing), and LLVM needs some
limit on the extent to which side effects are reordered. I don't
particulary care about fences, but if we don't respect them then what
do we respect? Only the front end truly knows the semantics of traps
and other side effects. To me fences are just an initial stand-in at
LLVM level for those code motion boundaries. More precise intrinsics
could be introduced eventually if need to move traps across fences.

I actually see fences as a proxy for potential inter-process
communication and I/O. It's important that any opaque library call
could contain a fence.

For example, you brought up the infinite loop case. In general, to
make an infinite loop externally observable, it needs to read from a
volatile. Doing that would naturally prevent traps from being hoisted
above the loop.
>> I've begun to think that may-unwind (or may fail guard) and
readonly
>> should be mutually exclusive. readonly refers to all system memory,
>> not just LLVM-visible memory. To achieve the effect of
>> "aliases-with-nothing-in-llvm" it's cleaner to use alias
analysis.
> 
> (I've put this on the bug as well:)
> 
> https://llvm.org/bugs/show_bug.cgi?id=18912#c3
> 
> ```
> (In reply to comment #2)
>> The test case in this bug report is fixed by
>> http://reviews.llvm.org/rL256728.
>> 
>> I'm closing this because I don't have a test case and I no
longer think it
>> makes much sense to mark may-unwind calls "readonly". An
unwind path always
>> touches some memory. That fact that it doesn't alias with
LLVM-visible
> 
> I'm not sure about this -- why does an unwind path *have* to write
> some memory? Can't you implement unwind as "read the appropriate
> exception handler RPC from a table, and return to that RPC"?
> ```
Just for the record, since the discussion has moved away from “readonly”…

I agree. As I said in the bug though, we should distinguish between
system memory and the memory addresses visible to LLVM. If a
personality function touches memory, the resume/unwind probably should not
be marked readonly even if it doesn't alias with anything in LLVM. That
said, I
think the more important issue is whether the unwindable calls can be
reordered. If not, I don't think they should be readonly.
>> memory access can be handled by alias analysis.
> 
> Btw, I think in this interpretation it is incorrect for -functionattrs
> to infer readnone for @foo in (which it does today):
> 
> define void @foo({ i8*, i32 } %val) personality i8 8 {
>  resume { i8*, i32 } %val
> }
From the PR: the -functionattrs behavior looks like a bug to me, strictly
speaking
(how can it assume the memory behavior of the personality function?)…
But maybe there’s a good reason for allowing this optimization.

-Andy

On Mon, Feb 22, 2016 at 9:40 PM, Andrew Trick <atrick at apple.com>
wrote:
> I actually see fences as a proxy for potential inter-process
> communication and I/O. It's important that any opaque library call
> could contain a fence.
This makes perfect sense to me now, especially if you want to use
@trap_on for safety checks.  Without re-ordering restrictions, a
failed @trap_on will end up looking a lot like UB (since, say, you
could reorder a failing range check across a call to fflush); and so
that would be bad.  You'd still have to be careful around
inaccessiblememonly and friends, though.

-- Sanjoy