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

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 1 09:54:07 PDT 2019


nickdesaulniers added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2650
   B.Reg = FuncInfo.CreateReg(B.RegVT);
   SDValue CopyTo = DAG.getCopyToReg(getControlRoot(), dl, B.Reg, Sub);
 
----------------
So it seems to me that `CopyTo` might not even end up as the root.  It looks like we have 3 cases that affect the root:
1. `!B.OmitRangeCheck`
2. `MBB != NextBlock(SwitchBB)`
3. Neither 1 or 2

For cases 1 and 2, we do all this wasteful work to create `CopyTo` and `Sub`, just to throw it away.  It seems like `RangeSub` isn't needed for case 2.  I think it would be clearer and do less work for each case to have a simple 3 arm conditional, rather than successive conditionals that may just throw away previous work to reassign to `Root`.


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

https://reviews.llvm.org/D68131





More information about the llvm-commits mailing list