[PATCH] Update optimization passes to handle inalloca arguments

Reid Kleckner rnk at google.com
Fri Jan 17 17:21:54 PST 2014


  A lot of this changed and I think the questionable parts went away.  PTAL


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

================
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);
----------------
Nick Lewycky wrote:
> "considered to be" --> "implicitly"? You get undef if you load from one afterwards, right?
Or just "are clobbered..."

================
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;
----------------
Nick Lewycky wrote:
> "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."
Done.

================
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;
----------------
Nick Lewycky wrote:
> Unclear what "static" means here.
It's unclear immediately after "isStaticAlloca()?"

================
Comment at: lib/IR/Instructions.cpp:935
@@ +934,3 @@
+    unsigned ArgNo = CS.getArgumentNo(UI);
+    if (CS.paramHasAttr(1 + ArgNo, Attribute::InAlloca))
+      return CS;
----------------
Nick Lewycky wrote:
> 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.
Interesting.  This code is gone now. =/

================
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;
+
----------------
Nick Lewycky wrote:
> Optional: Why do you need isUsedWithInAlloca() instead of just calling this and testing the result? CallSite evaluates to false when zero-initialized.
Gone now, but I thought it read better.


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



More information about the llvm-commits mailing list