[PATCH] Add a pass for constructing gc.statepoint sequences w/explicit relocations

Andrew Trick atrick at apple.com
Mon Feb 9 23:25:00 PST 2015


I agree with Sanjoy's comments. I'm not sure why you're opposed to cleaning up the code before submitting it for review:

- using LLVM's ADT containers
- following LLVM naming conventions (violated in a few places).
- following LLVM comment style!

Is it becase you want more feedback on the algorithm itself first?

You say you plan "The liveness algorithm improvements mentioned previously." Do you just mean the improvements listed above in the commit message?

There isn't really any liveness analysis here at all. The LiveVariables approach is pretty good for SSA liveness, so you could just steal it. From your comments I gather your saving that as a follow up improvement.

If you fix the obvious style issues, I don't really object to this being in trunk. We've already established that the pass will be on trunk (but not in the standard pipeline).

Once in trunk, if you want more feedback on certain aspects of the pass, whether design or implementation, you'll just have to be clear on what kind of review you want.


================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:163
@@ +162,3 @@
+    for (Type *SubType : ST->subtypes())
+      bad |= isGCPointerType(SubType) || isAggWhichContainsGCPtrType(SubType);
+    return bad;
----------------
This "bad" variable name is mysterious. In this scope, it doesn't seem "bad".

http://reviews.llvm.org/D6975

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






More information about the llvm-commits mailing list