Chandler Carruth via llvm-dev
2015-Sep-02 17:51 UTC
[llvm-dev] 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>
Sanjoy Das via llvm-dev
2015-Sep-02 20:41 UTC
[llvm-dev] RFC: Add "operand bundles" to calls and invokes
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
Chandler Carruth via llvm-dev
2015-Sep-03 01:04 UTC
[llvm-dev] RFC: Add "operand bundles" to calls and invokes
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>