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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

Thanks for the patch!  Just some small thoughts on style.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2663
 
-  SDValue BrRange = DAG.getNode(ISD::BRCOND, dl,
-                                MVT::Other, CopyTo, RangeCmp,
-                                DAG.getBasicBlock(B.Default));
-
-  // Avoid emitting unnecessary branches to the next block.
-  if (MBB != NextBlock(SwitchBB))
-    BrRange = DAG.getNode(ISD::BR, dl, MVT::Other, BrRange,
-                          DAG.getBasicBlock(MBB));
+  if (!B.OmitRangeCheck) {
+    SDValue BrRange = DAG.getNode(ISD::BRCOND, dl,
----------------
Please prefer:

```
  if true:
    foo()
  else:
    bar()
```
to
```
  if !true:
    bar()
  else
    foo()
```


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2670
+    if (MBB != NextBlock(SwitchBB))
+      BrRange = DAG.getNode(ISD::BR, dl, MVT::Other, BrRange,
+                            DAG.getBasicBlock(MBB));
----------------
If you declared a `SDValue` before the conditional, you could simply initialize it in these branches, and share the `setRoot` assignment after the conditional.


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

https://reviews.llvm.org/D68131