[PATCH] D39340: Modifying reassociate for improved CSE

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: include/llvm/Transforms/Scalar/Reassociate.h:76
+
+  unsigned Pass;
+  static const unsigned NumBinaryOps =
----------------
I find the use of this somewhat confusing. It seems to really be a loop counter stuck in the class to pass it to the other functions. Is it particularly inconvenient to pass this in to the functions where it's needed? Also a better name might be ReassociateStep or Level (possibly with an enum for the steps)? Pass is pretty overloaded (plus lldb doesn't understand what to do with this since there is also a class named Pass)


================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:2222
+          BestRank = MaxRank;
+          // fprintf(stderr,"%d\n", BestRank);
+        }
----------------
Leftover debug printing


Repository:
  rL LLVM

https://reviews.llvm.org/D39340