[PATCH] Update optimization passes to handle inalloca arguments

[Nick Lewycky]

================
Comment at: include/llvm/IR/Argument.h:64
@@ +63,3 @@
+  /// attribute on it in its containing function.  These attributes are very
+  /// similar from the perspective of the callee.
+  bool hasByValOrInAllocaAttr() const;
----------------
"the byval attribute or inalloca attribute"
or *the inalloca attribute?

"from the perspective of the callee" doesn't really tell the reader much (why are they similar?). Suggestion: "These attributes both represent arguments being passed by copy under different ABIs."

================
Comment at: include/llvm/IR/Instructions.h:35
@@ -34,2 +34,3 @@
 class LLVMContext;
+class ImmutableCallSite;
 
----------------
Sort.

================
Comment at: include/llvm/IR/Instructions.h:115
@@ +114,3 @@
+  /// isUsedWithInAlloca - Return true if this alloca is used as an inalloca
+  /// argument to a call.  Such allocas are never considered static.
+  bool isUsedWithInAlloca() const;
----------------
Unclear what "static" means here.

================
Comment at: include/llvm/IR/Instructions.h:120
@@ +119,3 @@
+  /// InAlloca attribute, or nothing.  There can be zero or one such call sites.
+  ImmutableCallSite getInAllocaCallSite() const;
+
----------------
Optional: Why do you need isUsedWithInAlloca() instead of just calling this and testing the result? CallSite evaluates to false when zero-initialized.

================
Comment at: lib/IR/Instructions.cpp:935
@@ +934,3 @@
+    unsigned ArgNo = CS.getArgumentNo(UI);
+    if (CS.paramHasAttr(1 + ArgNo, Attribute::InAlloca))
+      return CS;
----------------
No action required: By the way, this is actually O(n^2) not O(n) because attributes are stored in a linked list so finding 1+ArgNo is not constant time. See PR16536.

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:84
@@ -83,1 +83,3 @@
 
+      // Inalloca arguments are considered to be clobbered by the call.
+      unsigned ArgNo = CS.getArgumentNo(UI);
----------------
"considered to be" --> "implicitly"? You get undef if you load from one afterwards, right?


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