[PATCH] D39262: [CodeGen] Peel off the dominant case in switch statement in lowering

[Hans Wennborg via Phabricator via llvm-commits]

hans added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:9994
+  // Scale the branchprobability for DefaultMBB if the peel occurs and
+  // DefaultMBB is not replaced.
+  if (PeeledProb != BranchProbability::getOne() &&
----------------
xur wrote:
> hans wrote:
> > When would DefaultMBB get replaced? If it can, it needs a test case.
> We replace the unreachable default with most popular destination.  This actually exposed by an existing test case.
I still don't understand why it matters here if DefaultMBB has changed?


================
Comment at: test/CodeGen/Generic/switch-lower-peel-top-case.ll:12
+    i32 129, label %sw.bb7
+  ], !prof !2
+
----------------
hans wrote:
> This could probably be simplified a bit. If %n were an i32 instad of i16 you don't need the sext. You could drop local_unnamed_addr.
> 
> The case labels look pretty random; maybe chose cases with more purpose, and rename the target blocks to match. It would also be good to test the case where two adjacent cases are merged into a cluster and peeled.
The part about case values, and test for peeling two adjacent cases still applies.


https://reviews.llvm.org/D39262