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

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 18 09:19:59 PDT 2023

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...



More information about the llvm-commits mailing list