[PATCH] D40049: [PATCH] Global reassociation for improved CSE

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

A couple minor comments (feel free to address as you commit).



================
Comment at: include/llvm/Transforms/Scalar/Reassociate.h:77
+  // Arbitrary, but prevents quadratic behavior.
+  static const unsigned GlobalReassociateLimit = 10;
+  static const unsigned NumBinaryOps =
----------------
Can you make GlobalReassociateLimit a cl::opt (in case we feel like experimenting with tuning it at some point)?


================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:2244
+      // Collect all operands in a single reassociable expression.
+      // Since Reassociate has already been run once, we can assume things
+      // are already canonical according to Reassociation's regime.
----------------
This comment is now out of date (reassociate is not run first now, right?).


Repository:
  rL LLVM

https://reviews.llvm.org/D40049