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

[Jun Bum Lim via Phabricator via llvm-commits]

junbuml added inline comments.


================
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;
----------------
junbuml wrote:
> haicheng wrote:
> > 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. 
> I'm not sure if we really need to consider  the size of table as a cost. I think just couple of instructions to look up the table and jump to actual blocks need to be considered as cost.
You are right Haicheng. Looks like we need to consider the cost of the table as well. 


https://reviews.llvm.org/D31085