[PATCH] D77689: [X86] Codegen for preallocated

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks marked 4 inline comments as done.
aeubanks added inline comments.


================
Comment at: llvm/test/CodeGen/X86/musttail-indirect.ll:58
 
+; FIXME: This generates a lot of code even at -O2, any better way to do this? Same with all the preallocated versions of functions below.
+; CHECK-LABEL: g_thunk_2:
----------------
efriedma wrote:
> rnk wrote:
> > This is interesting, we forgot about this when writing the LangRef.
> > 
> > If you look at the previous inalloca test, it forwards its inalloca parameter directly to the musttail callee. It doesn't allocate new memory. That's the real use case for this `musttail` thing, and we probably need to bend the verifier rules to allow direct forwarding of preallocated arguments to a musttail call site.
> > 
> > I think for now, committing this as is with a FIXME and coming back to it later would be fine. I assume that the verifier rejects it if you remove the setup and arg calls.
> I don't think it makes sense to commit this test as-is.  The verifier should reject the combination of musttail and a preallocated bundle.  The llvm.call.preallocated.setup would have to allocate memory on top of the argument, and there's no reasonable way to prove that's safe.
> 
> And yes, we probably need to bend the rules for the "preallocated" attribute to allow forwarding arguments in musttail calls.
Added verifier check in https://reviews.llvm.org/D80132.
Is it ok to proceed with this and do the LangRef/codegen changes to support musttail and preallocated together in a later change?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77689/new/

https://reviews.llvm.org/D77689