[PATCH] D74651: Add IR constructs for inalloca replacement preallocated call setup

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.

I think in the end, the description of how this is lowered was left out, so this looks good and we can commit it as is.

However, I'd like to ensure that it is valid to replace preallocated call sites with static allocas (allocas in the entry block).

In D74651#2000661 <https://reviews.llvm.org/D74651#2000661>, @efriedma wrote:

> > I think if we model calls with preallocated bundles as modifying inaccessible state (i.e. the stack pointer), LLVM transforms won't do this.
>
> I don't think that's enough in general.  Well, maybe it's enough for the transforms we actually implement... LLVM currently has very few transforms that fold code together.
>
> But a simple example, suppose we had a transform that took `void g(); void f() { g();g();g();g();g(); }` and transformed it into something like `void g(); void f() { for (int i =0; i < n; ++i) g(); }`.  Looks fine generally.  If the loop body is a sequence containing calls to `llvm.call.preallocated.setup()`/`llvm.call.preallocated.arg()`, maybe not so fine.  (I think this is only an issue if there is no call actually consuming the stack; if there were, the token would block the transform.)


I don't think I understand this example. This looks like loop re-rolling to me, though, to give the transform a name.

If g() uses a preallocated call site, the loop re-roller would have to execute the exact same sequence of preallocated call intrinsics as it would in the unrolled form. That seems like it's all fine.

What about this transform rearranges the call site intrinsics to make them overlap, so that we would need two argument memory locations alive at the same time?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74651