[PATCH] [RewriteStatepointsForGC] Strip deref info after rewriting.

[Sanjoy Das]

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2226
@@ +2225,3 @@
+
+  auto RemoveDerefAttrAtIfNeeded = [&](unsigned Index) {
+    // Code common to handling return values and argument attributes in function
----------------
reames wrote:
> The IfNeeded doesn't add anything in terms of meaning.
Will fix.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2227
@@ +2226,3 @@
+  auto RemoveDerefAttrAtIfNeeded = [&](unsigned Index) {
+    // Code common to handling return values and argument attributes in function
+    // prototypes.
----------------
reames wrote:
> Comment should be outside lambda.  I'd be tempted to pull the lambda out as a static helper function since it's fairly large.
Moved out the lambda as a templated static helper.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2256
@@ +2255,3 @@
+
+  auto RemoveDerefAttrAtIfNeeded = [&](CallSite CS, unsigned Index) {
+    // Code common to handling return values and argument attributes in
----------------
reames wrote:
> Code duplication ahoy.  Could/should this be commoned with a templated helper function?
Done.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2275
@@ +2274,3 @@
+      assert(MD->getNumOperands() < 5 && "unrecognized metadata shape!");
+      bool IsImmutableTBAA =
+          MD->getNumOperands() == 4 &&
----------------
reames wrote:
> Is there a TBAA helper class we can use here?  Not sure there is, but please check.
Looked, could not find anything.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2277
@@ +2276,3 @@
+          MD->getNumOperands() == 4 &&
+          mdconst::extract<ConstantInt>(MD->getOperand(3))->getValue() == 1;
+
----------------
reames wrote:
> It looks like you're only handling one of two forms of TBAA metadata here...
AFAICT, LLVM's autoupgrade rewrites all TBAA metadata to the struct-path format (in `llvm::UpgradeInstWithTBAATag`).

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2297
@@ +2296,3 @@
+      if (isa<PointerType>(CS.getType()))
+        RemoveDerefAttrAtIfNeeded(CS, 0);
+    }
----------------
reames wrote:
> Use Atributes::ReturnIndex both here and above.  No more magic constants!
Will fix.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2375
@@ -2267,2 +2374,3 @@
   MadeChange |= insertParsePoints(F, DT, this, ParsePointNeeded);
+
   return MadeChange;
----------------
reames wrote:
> Whitespace?
Will fix.

http://reviews.llvm.org/D10105

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/