[PATCH] D31085: [InlineCost] Increase the cost of Switch

[Haicheng Wu via Phabricator via llvm-commits]

haicheng added inline comments.


================
Comment at: include/llvm/CodeGen/SwitchCaseCluster.h:123
+
+  /// Returns the estimated number of clusters.
+  unsigned getEstimatedNumberOfCluster(const SwitchInst &SI);
----------------
Return


================
Comment at: lib/Analysis/InlineCost.cpp:1006
 
-  // Otherwise, we need to accumulate a cost proportional to the number of
-  // distinct successor blocks. This fan-out in the CFG cannot be represented
-  // for free even if we can represent the core switch as a jumptable that
-  // takes a single instruction.
+  // Otherwise, we assume the most general case where the big swith is lowered
+  // into a balanced binary tree consisting of case clusters, the probability of
----------------
I think we can exit early if the number of cases is too large.


================
Comment at: lib/Analysis/InlineCost.cpp:1015
   // does not (yet) fire.
-  SmallPtrSet<BasicBlock *, 8> SuccessorBlocks;
-  SuccessorBlocks.insert(SI.getDefaultDest());
-  for (auto I = SI.case_begin(), E = SI.case_end(); I != E; ++I)
-    SuccessorBlocks.insert(I.getCaseSuccessor());
-  // Add cost corresponding to the number of distinct destinations. The first
-  // we model as free because of fallthrough.
-  Cost += (SuccessorBlocks.size() - 1) * InlineConstants::InstrCost;
+  int NumCaseCluster = TTI.getEstimatedNumberOfCaseClusters(SI);
+  SmallVector<unsigned, 4> SwitchWorkList;
----------------
If the estimation chooses to use jumptable, I think we also need to add the cost of the table which is proportional to the range. 


================
Comment at: lib/CodeGen/SelectionDAG/SwitchCaseCluster.cpp:86
+  APInt MinCaseVal = MaxCaseVal;
+  for (auto I = SI.case_begin(), E = SI.case_end(); I != E; ++I) {
+    const APInt &CaseVal = I.getCaseValue()->getValue();
----------------
We can start from begin()+1


================
Comment at: test/Transforms/Inline/switch.ll:3
 ; RUN: opt < %s -passes='cgscc(inline)' -inline-threshold=20 -S | FileCheck %s
 
+define i32 @callee1(i32 %a) {
----------------
We may need to add tests for jump table and bit test.


https://reviews.llvm.org/D31085