[PATCH] [SROA] Simplify PHI nodes before promoting the alloca

Eli Bendersky eliben at google.com
Thu Jul 24 11:35:55 PDT 2014


================
Comment at: lib/Transforms/Scalar/SROA.cpp:3101
@@ +3100,3 @@
+/// This function performs a standard depth-first search along the use-def
+/// chains of the PHI nodes. If the searching reaches a non-PHI other than
+/// Source, it stops and returns true.
----------------
by "of the PHI nodes" do you mean "PN"?

================
Comment at: lib/Transforms/Scalar/SROA.cpp:3102
@@ +3101,3 @@
+/// chains of the PHI nodes. If the searching reaches a non-PHI other than
+/// Source, it stops and returns true.
+static bool
----------------
Please document all the arguments of this function

================
Comment at: lib/Transforms/Scalar/SROA.cpp:3140
@@ +3139,3 @@
+  for (auto PN : PHIUsers) {
+    if (!PHINodeOriginatesFromOther(PN, Visited, OriginatesFromOther, NewAI))
+      ToDelete.insert(PN);
----------------
Does the logic have to be inverted? If you only use this method in a "!" clause, can't it check positively - i.e. origination from only NewAI? This can make reasoning about it simpler.

================
Comment at: lib/Transforms/Scalar/SROA.cpp:3143
@@ +3142,3 @@
+  }
+  for (auto PN: ToDelete) {
+    PHIUsers.erase(PN);
----------------
whitespace around ":"

http://reviews.llvm.org/D4659






More information about the llvm-commits mailing list