[PATCH] D92159: [LSR][NFC] don't collect chains when isNumRegsMajorCostOfLSR is false

[ChenZheng via Phabricator via llvm-commits]

shchenz created this revision.
shchenz added reviewers: samparker, jsji, hfinkel, qcolombet.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
shchenz requested review of this revision.

This is an NFC patch and it only concerns compiling time on targets where `isNumRegsMajorCostOfLSR` is overloaded. For now, it is only for PowerPC target.

Because all the collected chains will be marked as not profitable in `isProfitableChain` when `isNumRegsMajorCostOfLSR` returns false, we don't need to waste time to collect the chain at the very beginning.

This patch just reverts the change in function `isProfitableChain` in https://reviews.llvm.org/D89665 and moves the newly added hook check at the very beginning.

We may get a potential issue if another hook `isProfitableLSRChainElement` returns true for some chain element on targets that `isNumRegsMajorCostOfLSR` returns false, but for now I can not see any target do that. So I guess it should be ok now?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92159

Files:
  llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp


Index: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -2860,20 +2860,12 @@
   unsigned NumVarIncrements = 0;
   unsigned NumReusedIncrements = 0;
 
-  // If any LSRUse in the chain is marked as profitable by target, mark this
-  // chain as profitable.
-  for (const IVInc &Inc : Chain.Incs)
-    if (TTI.isProfitableLSRChainElement(Inc.UserInst))
-      return true;
-
-  // If number of registers is not the major cost, we cannot benefit from this
-  // profitable chain which is based on number of registers.
-  // FIXME: add profitable chain optimization for other kinds major cost, for
-  // example number of instructions.
-  if (!TTI.isNumRegsMajorCostOfLSR())
-    return false;
+  if (TTI.isProfitableLSRChainElement(Chain.Incs[0].UserInst))
+    return true;
 
   for (const IVInc &Inc : Chain) {
+    if (TTI.isProfitableLSRChainElement(Inc.UserInst))
+      return true;
     if (Inc.IncExpr->isZero())
       continue;
 
@@ -5637,7 +5629,13 @@
   }
 
   // Start collecting data and preparing for the solver.
-  CollectChains();
+  // If number of registers is not the major cost, we cannot benefit from the
+  // current profitable chain optimization which is based on number of
+  // registers.
+  // FIXME: add profitable chain optimization for other kinds major cost, for
+  // example number of instructions.
+  if (TTI.isNumRegsMajorCostOfLSR() || StressIVChain)
+    CollectChains();
   CollectInterestingTypesAndFactors();
   CollectFixupsAndInitialFormulae();
   CollectLoopInvariantFixupsAndFormulae();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92159.307789.patch
Type: text/x-patch
Size: 1708 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201126/e81cb7b7/attachment.bin>