[PATCH] D109103: [ISEL][BitTestBlock] omit additional bit test when default destination is unreachable

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/lib/CodeGen/SwitchLoweringUtils.cpp:455
+  bool DefaultUnreachable =
+      isa<UnreachableInst>(SI->getDefaultDest()->getFirstNonPHIOrDbg());
   BitTestCases.emplace_back(std::move(LowBound), std::move(CmpRange),
----------------
hans wrote:
> I don't think this line is quite right.
> 
> The "default" for a bit test is not necessarily the same as the default for the SwitchInst -- it's just the block that control falls through to if all the bit tests fail, and that block could be one that handles other switch cases. (It would be better if BitTestBlock.Default was renamed to BitTestBlock.Fallthrough).
> 
> You can see how BitTestBlock.Default gets set in SelectionDAGBuilder::lowerWorkItem():
> 
> ```
>         // Fill in fields of the BitTestBlock.                          
>         BTB->Parent = CurMBB;
>         BTB->Default = Fallthrough;
> ```
> 
> However, that code does know whether the fallthrough block is unreachable, and already uses that to set the BitTestBlock::OmitRangeCheck. Maybe OmitRangeCheck could be renamed to UnreachableDefault, and used for both omitting the range check and skipping the last bit test? 
Oh, right, good point. Yeah, `OmitRangeCheck` is set based on the same criteria, I should just re-use that; then I don't need to add anything to `BitTestBlock`.

I think the identifier `OmitRangeCheck` is nice and explains what we're going to do, so I wont rename it to `UnreachableDefault`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109103