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

[Sanjoy Das via llvm-dev]

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