[PATCH] D106056: [CVP] processSwitch: Remove default case when switch cover all possible values.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:397-399
+      Low = APIntOps::smin(SIRange.getSignedMin(), Low);
+      High = APIntOps::smax(SIRange.getSignedMax(), High);
+      if (Low + (SI->getNumCases() - 1) == High) {
----------------
This logic seems more confusing to me than something like this:
      // If the numbered switch cases cover the entire range of the condition,
      // then the default case is not reachable.  
      if (SIRange.getSignedMin() == Low && SIRange.getSignedMax() == High &&
          SI->getNumCases() == High - Low + 1) {


We should have a negative test where High/Low match, but we don't have enough cases to cover all values. For example, remove `case 2` in the test that is included currently.


================
Comment at: llvm/test/Transforms/CorrelatedValuePropagation/basic.ll:396-397
+; CHECK-NEXT:    unreachable
+; CHECK:       unreachable1.split:
+; CHECK-NEXT:    br label [[UNREACHABLE:%.*]]
 ; CHECK:       unreachable:
----------------
Why do we create this dead block?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106056