[PATCH] D126043: [LSR] Drop LSR solution if it is less profitable than baseline

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

Is it feasible to add a test case?



================
Comment at: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp:3372
+    F.initialMatch(S, L, SE);
+    if (!BaselineCost.isLoser())
+      BaselineCost.RateFormula(F, Regs, VisitedRegs, LU);
----------------
How can the baseline immediately become the loser?


================
Comment at: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp:3373
+    if (!BaselineCost.isLoser())
+      BaselineCost.RateFormula(F, Regs, VisitedRegs, LU);
+
----------------
Since SCEV subexpressions can be reused by multiple instructions, doesn't this mean that this is overestimating the baseline cost?


================
Comment at: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp:5159
+
+  if (!SolutionCost.isLess(const_cast<Cost &>(BaselineCost))) {
+    LLVM_DEBUG(dbgs() << "\n"
----------------
Instead of the const cast, would it be possible to make `Cost::isLess` accept a const object?


================
Comment at: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp:5160
+  if (!SolutionCost.isLess(const_cast<Cost &>(BaselineCost))) {
+    LLVM_DEBUG(dbgs() << "\n"
+                         "The baseline solution requires ";
----------------
Starting with newline?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126043