[PATCH] Add the llvm.frameallocate and llvm.recoverframeallocation intrinsics

[Reid Kleckner]

Thanks! I'll upload the doc tweaks and DCE.

================
Comment at: docs/LangRef.rst:7238
@@ +7237,3 @@
+
+The '``llvm.frameallocate``' intrinsic allocates stack memory at some fixed
+offset from the frame pointer, and the '``llvm.recoverframeallocation``'
----------------
andrew.w.kaylor wrote:
> You should say something about how (or at least where) this "fixed offset" is determined.
Sure.

================
Comment at: docs/LangRef.rst:7246
@@ +7245,3 @@
+
+The ``size`` argument to '``llvm.frameallocate``' must be a constant integer
+indicating the amount of stack memory to allocate.
----------------
andrew.w.kaylor wrote:
> What happens if 'size' is zero?
Same thing as with normal allocas, which is apparently: Allocating zero bytes is legal, but the result is undefined.

================
Comment at: docs/LangRef.rst:7263
@@ +7262,3 @@
+These intrinsics allow a group of functions to share the stack memory
+allocation of an ancestor stack frame. The memory returned from
+'``llvm.frameallocate``' is allocated prior to stack realignment to produce a
----------------
andrew.w.kaylor wrote:
> The phrase "the stack memory allocation" is a bit ambiguous.  You only mean for it to refer to the part of stack memory allocated by this intrinsic, right?  So if the function contains other allocas is addition to this intrinsic the descendant functions can't access that alloca (without rolling it into this frame allocation), yes?
Yeah, I should probably say "one stack memory allocation".

================
Comment at: docs/LangRef.rst:7267
@@ +7266,3 @@
+ABI-required stack alignment.  Each function may only call
+'``llvm.frameallocate``' one or zero times from the function entry block.  The
+frame allocation intrinsic inhibits inlining, as any frame allocations in the
----------------
andrew.w.kaylor wrote:
> Can the call be anywhere in the entry block?  Can there be allocas before or after this call?
Anywhere. The same is true for normal allocas. Hypothetically you could call another function that recovers the allocation prior to making the allocation, but that seems like obvious UB not worth mentioning.

================
Comment at: docs/LangRef.rst:7271
@@ -7224,1 +7270,3 @@
+by '``llvm.recoverframeallocation``' called with the uninlined function.
+
 .. _int_read_register:
----------------
andrew.w.kaylor wrote:
> I think it would be helpful to describe the exception handling case here as an example of why this is useful.  With everything kept abstract like this it is hard to understand why anyone would want to do this.
I think that should probably go in the ExceptionHandling document, not LangRef. I suppose I can write something up.

================
Comment at: lib/CodeGen/MachineFunction.cpp:594
@@ +593,3 @@
+  setFrameAddressIsTaken(true);
+  Size = RoundUpToAlignment(Size, StackAlignment);
+  FrameAllocationSize = Size;
----------------
andrew.w.kaylor wrote:
> Will this round zero up to a non-zero value?  If not, then the call to setFrameAddressIsTaken may be incorrect.  Also, it seems like a good idea to verify here that FrameAllocationSize is zero before it is set by this function.
I don't think it will round zero up.

Actually, FrameAllocationSize is dead, I just forgot to delete it...

================
Comment at: lib/IR/Verifier.cpp:2584
@@ +2583,3 @@
+            "argument must be function defined in this module", &CI);
+    break;
+  }
----------------
andrew.w.kaylor wrote:
> Is there a reasonable way to verify that fp is at least a pointer?  Looking at your test case made me think that at the very least null should be flagged as an error, and it seems like any constant is likely to be an error.
This should already be handled automatically in VerifyIntrinsicType based on the types in Intrinsics.td.

================
Comment at: test/Verifier/frameallocate.ll:5
@@ +4,3 @@
+declare i8* @llvm.recoverframeallocation(i8*, i8*)
+
+define internal void @f() {
----------------
andrew.w.kaylor wrote:
> Checking for expected behavior if llvm.frameallocate is called with a zero argument would be nice.  You should also have a test for a non-constant argument.
Sure, non-constant test makes sense. I'm not particularly interested in the zero size case, as long as making it UB is OK.

================
Comment at: test/Verifier/frameallocate.ll:33
@@ +32,3 @@
+define internal void @i() {
+  call i8* @llvm.recoverframeallocation(i8* @global, i8* null)
+  ret void
----------------
andrew.w.kaylor wrote:
> The second argument here looks like an error also.
Passing null here is just UB at runtime, not a verifier error. This code might never be executed, but it should assemble.

http://reviews.llvm.org/D6493