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

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 27 07:48:38 PDT 2019


spatel 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);
----------------
Please pre-commit this and other cosmetic diffs.


================
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
----------------
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?


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

https://reviews.llvm.org/D68131





More information about the llvm-commits mailing list