llvm dev - Sep 2015 - RFC: Add "operand bundles" to calls and invokes

Just wanted to confirm that I too like where this is going. =] I think
Philip and others have really handled the bulk of the review, and I'm very
comfortable with them finishing the patch review.

One issue where I wanted to chime in, hopefully just to add some clarity,
is the "readonly" vs operand bundle set of (interrelated) issues.

First, as I think Philip already said, I think it is important that a
readonly or a readnone attribute on a call is absolute. Optimizations
shouldn't have to go look for an operand bundle. Instead, we should prevent
the call-side attributes from being added.

I think there may be a separate way of specifying all of this that makes
things clearer. Operand bundles imply that when lowering, the call may be
wrapped with a call to an external function before and/or after the called
function, with the bundled operands escaped into those external functions
which may capture, etc.

This both gives you the escape semantics, and it gives you something else;
the runtime function might not return! That should (I think) exactly
capture the semantic issue you were worried about with deopt. Because
control may never reach the called function, or may never return to the
caller even if the callee returns, code motion of side-effects would be
clearly prohibited.

Does this make sense as an approach to specifying things? (Or worse, are
you already there, and I'm just arriving late to the party?)

-Chandler

On Fri, Aug 28, 2015 at 4:42 PM Sanjoy Das via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Initial set of patches are up for review at:
>
> http://reviews.llvm.org/D12455
> http://reviews.llvm.org/D12456
> http://reviews.llvm.org/D12457
>
> Thanks,
> -- Sanjoy
> _______________________________________________
> 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/20150902/d07af204/attachment.html>

Hi Chandler,

Thanks for replying!
> First, as I think Philip already said, I think it is important that a
> readonly or a readnone attribute on a call is absolute. Optimizations
> shouldn't have to go look for an operand bundle. Instead, we should
prevent
> the call-side attributes from being added.
I think Philip's concern was more about the *difference* between the
call side attributes and attributes on the function.

Say you have

define i32 @f() {
  ret i32 42
}

define void @g() {
  call void @f() [ "foo"(i32 100) ]
  ret void
}

Now I think we all agree that the call to `@f` cannot be marked as
`readnone` to have deopt semantics.  We can (I suspect without too
much churn) make sure LLVM does not take such a `call` and mark it as
`readnone`.

However, `-functionattrs` (and related passes) are still allowed to
mark the *function* (`@f`) as `readnone`, and I think it would be very
weird if we disallowed that (since we'll have to iterate through all
of `@f`'s uses).

This brings us to the weird situation where we can have a
not-`readnone` call to a function that's marked `readnone`.  This was
Philip's concern -- the semantics of the call is no longer the most
precise that can be deduced by looking at both the call and function
attributes.  We'd possibly have issues with passes that looked at the
`CS.getCalledFunction()`'s attributes and decided to do an illegal
reordering because the function was marked `readnone`.
> I think there may be a separate way of specifying all of this that makes
> things clearer. Operand bundles imply that when lowering, the call may be
> wrapped with a call to an external function before and/or after the called
> function, with the bundled operands escaped into those external functions
> which may capture, etc.
>
> This both gives you the escape semantics, and it gives you something else;
> the runtime function might not return! That should (I think) exactly
capture
> the semantic issue you were worried about with deopt. Because control may
> never reach the called function, or may never return to the caller even if
> the callee returns, code motion of side-effects would be clearly
prohibited.
This is sort of what I was getting at when I said

"As a meta point, I think the right way to view operand bundles is as
something that *happens* before and after an call / invoke, not as a
set of values being passed around."

But with this scheme, the issue with a function's attributes being out
of sync with its actual semantics at a call site still exists.

I think a reasonable specification is to add a function attribute
`may_deopt_caller`[1].  Only functions that are marked
`may_deopt_caller` can actually access the operand bundles that was
passed to the function at a call site, and `may_deopt_caller` implies
all of the reordering restrictions we are interested in.
`-functionattrs` is not allowed to mark a `may_deopt_caller` function
as `readnone` (say) because they're not.  If we wanted to be really
clever, we could even DCE deopt operand bundles in calls to functions
that are not marked `may_deopt_caller`.

This does bring up the semantic issue of whether `may_deopt_caller` is
truly a property of the callee, or am I just trying to come up with
arbitrary conservative attributes to sweep a complex issue under the
carpet.  I'll have to spend some time thinking about this, but at this
time I think it is the former (otherwise I wouldn't be writhing this
:)) -- typically a callee has to *do* something to deopt its caller,
and that's usually a call to the runtime.  `may_deopt_caller` in this
case is a conservative attribute stating that the callee may execute
such a deopting call.  The most similar existing attribute I can find
is `returns_twice`.

It is (conservatively) okay to mark any function with
`may_deopt_caller`; and if LLVM's only concern was compiling for
managed, deopting environments, I'd consider making `may_deopt_caller`
the default and have an attribute `does_not_deopt` to indicate it's
negation.  `does_not_deopt` would then be closer to more common
attributes like `readonly` and `argmemonly` -- its presence makes
optimization more effective, and its absence is conservatively
correct.

[1]: We can call it something more generic too, like
`inspects_stack_state` etc.  That bike shed will be painted later.

-- Sanjoy

On Wed, Sep 2, 2015 at 1:42 PM Sanjoy Das <sanjoy at
playingwithpointers.com>
wrote:
> Hi Chandler,
>
> Thanks for replying!
>
> > First, as I think Philip already said, I think it is important that a
> > readonly or a readnone attribute on a call is absolute. Optimizations
> > shouldn't have to go look for an operand bundle. Instead, we
should
> prevent
> > the call-side attributes from being added.
>
> I think Philip's concern was more about the *difference* between the
> call side attributes and attributes on the function.
>
> Say you have
>
> define i32 @f() {
>   ret i32 42
> }
>
> define void @g() {
>   call void @f() [ "foo"(i32 100) ]
>   ret void
> }
>
> Now I think we all agree that the call to `@f` cannot be marked as
> `readnone` to have deopt semantics.  We can (I suspect without too
> much churn) make sure LLVM does not take such a `call` and mark it as
> `readnone`.
>
> However, `-functionattrs` (and related passes) are still allowed to
> mark the *function* (`@f`) as `readnone`, and I think it would be very
> weird if we disallowed that (since we'll have to iterate through all
> of `@f`'s uses).
>
> This brings us to the weird situation where we can have a
> not-`readnone` call to a function that's marked `readnone`.  This was
> Philip's concern -- the semantics of the call is no longer the most
> precise that can be deduced by looking at both the call and function
> attributes.  We'd possibly have issues with passes that looked at the
> `CS.getCalledFunction()`'s attributes and decided to do an illegal
> reordering because the function was marked `readnone`.
>
While I'm still mulling it over, I think that if we want something like
operand bundles, we really need to move to the point where the *only* valid
set of attributes to query is the call attributes when trying to understand
the semantics of a call instruction. I actually like this model better. It
clearly separates the idea that a particular call instruction's semantics
are modeled by a particular call instruction attribute set. A particular
function's semantics are modeled by *its* attribute set. Depending on the
nature of the query, you should look at different ones.

Historically, getting this wrong only manifested in missed optimizations.
With the ability to add extra functionality to call instructions (outside
of the called function) we inherently introduce the concept of this being a
correctness issue. I think we'll have to carefully audit the optimizer
here, but I'm not (yet) too worried about the ramifications.

> > I think there may be a separate way of specifying all of this that
makes
> > things clearer. Operand bundles imply that when lowering, the call may
be
> > wrapped with a call to an external function before and/or after the
> called
> > function, with the bundled operands escaped into those external
functions
> > which may capture, etc.
> >
> > This both gives you the escape semantics, and it gives you something
> else;
> > the runtime function might not return! That should (I think) exactly
> capture
> > the semantic issue you were worried about with deopt. Because control
may
> > never reach the called function, or may never return to the caller
even
> if
> > the callee returns, code motion of side-effects would be clearly
> prohibited.
>
> This is sort of what I was getting at when I said
>
> "As a meta point, I think the right way to view operand bundles is as
> something that *happens* before and after an call / invoke, not as a
> set of values being passed around."
>
> But with this scheme, the issue with a function's attributes being out
> of sync with its actual semantics at a call site still exists.
>
> I think a reasonable specification is to add a function attribute
> `may_deopt_caller`[1].  Only functions that are marked
> `may_deopt_caller` can actually access the operand bundles that was
> passed to the function at a call site, and `may_deopt_caller` implies
> all of the reordering restrictions we are interested in.
> `-functionattrs` is not allowed to mark a `may_deopt_caller` function
> as `readnone` (say) because they're not.  If we wanted to be really
> clever, we could even DCE deopt operand bundles in calls to functions
> that are not marked `may_deopt_caller`.
>
> This does bring up the semantic issue of whether `may_deopt_caller` is
> truly a property of the callee, or am I just trying to come up with
> arbitrary conservative attributes to sweep a complex issue under the
> carpet.  I'll have to spend some time thinking about this, but at this
> time I think it is the former (otherwise I wouldn't be writhing this
> :)) -- typically a callee has to *do* something to deopt its caller,
> and that's usually a call to the runtime.  `may_deopt_caller` in this
> case is a conservative attribute stating that the callee may execute
> such a deopting call.  The most similar existing attribute I can find
> is `returns_twice`.
>
I really think this just happens to be the special case of deopt, and that
it is a mistake to design the IR extension based solely on that use case.

Consider many of the other decorator patterns that have been discussed as
uses of this IR functionality. If the runtime logic invoked before or after
the function can read or write memory other than what the callee does, we
are moving to a point where the call instruction's annotations
(attributes + operand bundles) introduce a *more restrictive* semantic
model than the function attributes alone.

I'm actually much more comfortable with the highly generic approach and
eating the cost of teaching the optimizer about this distinction.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150903/90803e5c/attachment.html>