[PATCH] D146903: [SimplifyCFG] add nsw on SwitchToLookupTable Index calculation

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/test/Transforms/SimplifyCFG/X86/switch-table-bug.ll:12
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[SWITCH_TABLEIDX:%.*]] = sub i2 [[TMP0:%.*]], -2
+; CHECK-NEXT:    [[SWITCH_TABLEIDX:%.*]] = sub nsw i2 [[TMP0:%.*]], -2
 ; CHECK-NEXT:    [[SWITCH_TABLEIDX_ZEXT:%.*]] = zext i2 [[SWITCH_TABLEIDX]] to i3
----------------
nikic wrote:
> This is a miscompile: E.g. if the input was 0 then we'll do 0 - (-2) which should be 2 but overflows to -2.
> 
> The bit I missed here is that we're effectively mapping a signed range to an unsigned one, doing something like -2 -> 0, -1 -> 1, 0 -> 2, 1 -> 3, but of course the last two become -2 and -1 when interpreted as signed.
> 
> I think we need an additional check that TableSize is small enough (should not be larger than half the integer space, or something like that).
Probably the simplest way to check this is to do something like `MaxCaseVal->getValue().ssub_ov(MinCaseVal->getValue())` and check that it doesn't overflow. That directly corresponds to the operation we're doing...


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

https://reviews.llvm.org/D146903