[PATCH] D10631: [Inliner][NFCI] Add an InlineSite abstraction.

[Sanjoy Das]

sanjoy added a comment.

Hi Chandler,

What is your current stance on this?  Should I start making `gc_statepoint` calls / invokes managed by `CallSite`?

The only potential downside I see to making `CallSite`s manage `gc_statepoint` is that it will make `CallSite` more complex than a simple dispatch over `CallInst` vs. `InvokeInst`.  `CallSite` will now have to dispatch over the cross product of {`CallInst`, `InvokeInst`} and { `isStatepoint`, `!isStatepoint` }.  There will be a slight performance penalty[1] and some bits of LLVM that depend on `CallSite` being uncomplicated may require updating.  I don't mind doing all of this if we can all agree that `CallSite` managed statepoints are the right way to do this.

<hand-waving>

Here is how I think about what's going on, in case that is helpful:

The mental model I have for inlining through statepoints is that the inliner "sees through" some control and data flow in the gc_statepoint intrinsic and inlines two levels at once.  It can then inline through / simplify the attached `gc_relocate` and `gc_result` calls as well.

For instance, an imaginary implementation of `gc_statepoint` that will allow the current (i.e. implemented in the current patch set) form of inlining looks like:

  define i32 @gc_statepoint(func_ptr %f, ...args) {
    (arg0, arg1, ...) = unpack(...args) ;; can't be represented in LLVM IR
    %result = %f(arg0, arg1, ...)
    ret pack(%result)  ;; can't be easily represented in LLVM IR
  }

with the axiom

  gc_result(pack(%result)) == %result

The inliner then, semantically, first inlines through the `gc_statepoint` call, then recognizes that the call to `%f` is now direct, and then inlines through that call as well.  It then simplifies the associated `gc_(result|relocate)` calls[2].

There are two places where this view of things break down:

1. the "internals" of `@gc_statepoint` are not representable in LLVM IR, especially once we take into account deoptimization and gc state.
2. even if (1) was somehow addressed, it won't be okay to inline only through `@gc_statepoint` and not subsequently inline through `%f`.

But semantically, this is one way to think about what's going on.

</hand-waving>

[1]: Most of this may be mitigated by s/PointerIntPair<InstrTy*, 1, bool>/PointerIntPair<InstrTy*, 2> in `CallSite`
[2]: Currently we only simplify gc_result calls, logic simplification for gc_relocate will come later)


http://reviews.llvm.org/D10631