[PATCH] D68131: Switch lowering: omit range check for bit tests when default is unreachable (PR43129)

[Hans Wennborg via Phabricator via llvm-commits]

hans marked 6 inline comments as done.
hans added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2622
 
-  // Subtract the minimum value
+  // Subtract the minimum value.
   SDValue SwitchOp = getValue(B.SValue);
----------------
spatel wrote:
> Please pre-commit this and other cosmetic diffs.
r373191.


================
Comment at: llvm/test/CodeGen/X86/switch-bt.ll:161-179
+declare void @g(i32)
+define void @test5(i32 %x) {
+
+; CHECK-LABEL: test5
+; CHECK-NOT: cmp
+
+; 73 = 2^0 + 2^3 + 2^6
----------------
spatel wrote:
> Please add this test with baseline CHECK lines as a pre-commit for this patch, so we show the functional difference from this patch.
> 
> Does this test cover both sides of the 'if' in the new code?
> 
>     (MBB != NextBlock(SwitchBB))
> and
>     (MBB == NextBlock(SwitchBB))
> 
> If not, can you add another test to provide that coverage?
Pre-committed a test in r373190.


I think constructing a test where MBB != NextBlock(SwitchBB) would be very tricky.


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

https://reviews.llvm.org/D68131