[PATCH] Begin adding docs and IR-level support for the inalloca attribute

Reid Kleckner rnk at google.com
Mon Nov 25 15:27:00 PST 2013


  Thanks for the review!


================
Comment at: docs/LangRef.rst:704
@@ +703,3 @@
+
+.. Warning:: This feature is unstable and not fully implemented.
+
----------------
Sean Silva wrote:
> You may want to consider breaking this out into a separate page while it is still unstable, like Andy did for the stackmap/patchpoint stuff (splitting it into its own page seems to have been a huge win there, as the document can go into more depth about certain things). The Sphinx Quickstart Template makes it super easy to do this <http://llvm.org/docs/SphinxQuickstartTemplate.html>.
I'd already considered and rejected that, but I changed my mind.  I added one.  It's far from complete.  I'd like to develop this feature in the tree once we can reach agreement on at least the 'inalloca' attribute itself.

================
Comment at: docs/LangRef.rst:751
@@ +750,3 @@
+    invoke.cont:
+      call void @f(%Foo* inalloca %arg)
+      call void @llvm.stackrestore()
----------------
Sean Silva wrote:
> Is this meant to be a recursive invocation? (this BB seems to be in a function `define void @f`)
Oops, should be g.

================
Comment at: docs/LangRef.rst:750-756
@@ +749,9 @@
+
+    invoke.cont:
+      call void @f(%Foo* inalloca %arg)
+      call void @llvm.stackrestore()
+      ...
+
+    invoke.unwind:
+      call void @llvm.stackrestore()
+      ...
----------------
Sean Silva wrote:
> You aren't passing an argument to llvm.stackrestore
Woops, should be %base.

================
Comment at: docs/LangRef.rst:720
@@ +719,3 @@
+
+    Any use of ``inalloca`` arguments must comply with the target's ABI.
+    ``inalloca`` cannot be applied to parameters that are passed in
----------------
Sean Silva wrote:
> The connection between IR-level stuff and the actual machine code that will be emitted for these sorts of delicate ABI things is really quite tenuous, so I would avoid speaking in generalities about complying with a target ABI.
> 
> I would suggest providing explicit examples of ABI's and generated machine code and how an inalloca complies/doesn't comply with the ABI.
I edited this out.

The rules are pretty simple.  The argument has to be passed in memory, and the allocas have to be right to left.  The ABI comment was really just trying to motivate why the rules exist.

================
Comment at: docs/LangRef.rst:730-733
@@ +729,6 @@
+
+    To avoid stack leaks, frontends should free memory used for
+    ``inalloca`` calls with :ref:`llvm.stacksave <int_stacksave>` and
+    :ref:`llvm.stackrestore <int_stackrestore>`.  This can be done after
+    a call or on an unwind edge, as in the following example.
+
----------------
Sean Silva wrote:
> This really needs more explanation. You need to explicitly specify the circumstances under which a stack leak may or may not occur.
The stack leak follows directly from having dynamic allocas that aren't explicitly freed with stack restoration.  I elaborated on this in the standalone document.


http://llvm-reviews.chandlerc.com/D2173



More information about the llvm-commits mailing list