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

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/docs/LangRef.rst:1069
+    The preallocated attribute requires a type argument, which must be
+    the same as the pointee type of the argument.
 .. _attr_inalloca:
----------------
efriedma wrote:
> Are there any rules for what value has to be passed to a "preallocated" argument?  Or is the value ignored?
It's ignored, clarified.


================
Comment at: llvm/docs/LangRef.rst:11939
+corresponding argument. The token must be the parameter to a
+``"preallocated"`` operand bundle for the corresponding call.
+
----------------
efriedma wrote:
> Probably worth mentioning the rules for nested llvm.call.preallocated.setup .
Added blurb about how

t1 = setup()
t2 = setup()
foo() [t2]
foo() [t1]

is ok but not


t1 = setup()
t2 = setup()
foo() [t1]
foo() [t2],

is that good enough?


================
Comment at: llvm/docs/LangRef.rst:11963
+the call associated with the ``%setup_token``, which must be from
+'``llvm.call.preallocated.setup``'.
+
----------------
efriedma wrote:
> Maybe worth mentioning what happens if you call llvm.call.preallocated.arg after the memory is deallocated?
Said it's UB if you call llvm.call.preallocated.arg after the call, or after another llvm.call.preallocated.setup.


================
Comment at: llvm/lib/IR/Verifier.cpp:4522
+  }
   case Intrinsic::gcroot:
   case Intrinsic::gcwrite:
----------------
efriedma wrote:
> Do you need to check that there aren't any calls to llvm.call.preallocated.arg with the wrong token?
Good catch, I missed that. Done and added test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74651