[llvm-dev] RFC: Add "operand bundles" to calls and invokes

[Chandler Carruth via llvm-dev]

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>