[PATCH] D86050: [SCEV] Refactor isHighCostExpansionHelper

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:2184-2195
+  // In this polynominal, we may have some zero operands, and we shouldn't
+  // really charge for those. So how many non-zero coeffients are there?
+  int NumTerms = llvm::count_if(S->operands(), [](const SCEV *Op) {
+                                  return !Op->isZero();
+                                });
+
+  // Ignoring constant term (operand 0), how many of the coeffients are u> 1?
----------------
Move this back into `case scAddRecExpr:` please


================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:2218-2220
+  switch (S->getSCEVType()) {
+  default:
+    return 0;
----------------
I'm sorry, but i don't like this.
The old code would loudly fail should anyone introduce a new SCEV type and forget to update this,
new code will silently treat such a case as free.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86050/new/

https://reviews.llvm.org/D86050