Skip to content

[SimplifyCFG] Replace unreachable switch lookup table holes with poison #94990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 25, 2024
Merged

[SimplifyCFG] Replace unreachable switch lookup table holes with poison #94990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac23a90
[SimplifyCFG] Lookup table test cases
DaMatrix Jun 10, 2024
f9d54a5
[SimplifyCFG] Replace unreachable switch lookup table holes with poison
DaMatrix Dec 18, 2024
659c676
Update llvm/lib/Transforms/Utils/SimplifyCFG.cpp
DaMatrix Dec 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions llvm/lib/Transforms/Utils/SimplifyCFG.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6531,8 +6531,8 @@ SwitchLookupTable::SwitchLookupTable(
uint64_t Idx = (CaseVal->getValue() - Offset->getValue()).getLimitedValue();
TableContents[Idx] = CaseRes;

if (CaseRes != SingleValue)
SingleValue = nullptr;
if (SingleValue && !isa<PoisonValue>(CaseRes) && CaseRes != SingleValue)
SingleValue = isa<PoisonValue>(SingleValue) ? CaseRes : nullptr;
}

// Fill in any holes in the table with the default result.
Expand All @@ -6545,7 +6545,10 @@ SwitchLookupTable::SwitchLookupTable(
TableContents[I] = DefaultValue;
}

if (DefaultValue != SingleValue)
// If the default value is poison, all the holes are poison.
bool DefaultValueIsPoison = isa<PoisonValue>(DefaultValue);

if (DefaultValue != SingleValue && !DefaultValueIsPoison)
SingleValue = nullptr;
}

Expand All @@ -6569,6 +6572,16 @@ SwitchLookupTable::SwitchLookupTable(
// Check if there is the same distance between two consecutive values.
for (uint64_t I = 0; I < TableSize; ++I) {
ConstantInt *ConstVal = dyn_cast<ConstantInt>(TableContents[I]);

if (!ConstVal && isa<PoisonValue>(TableContents[I])) {
// This is an poison, so it's (probably) a lookup table hole.
// To prevent any regressions from before we switched to using poison as
// the default value, holes will fall back to using the first value.
// This can be removed once we add proper handling for poisons in lookup
// tables.
ConstVal = dyn_cast<ConstantInt>(Values[0].second);
}

if (!ConstVal) {
// This is an undef. We could deal with it, but undefs in lookup tables
// are very seldom. It's probably not worth the additional complexity.
Expand Down Expand Up @@ -7003,16 +7016,16 @@ static bool switchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder,

// If the table has holes but the default destination doesn't produce any
// constant results, the lookup table entries corresponding to the holes will
// contain undefined values.
bool AllHolesAreUndefined = TableHasHoles && !HasDefaultResults;
// contain poison.
bool AllHolesArePoison = TableHasHoles && !HasDefaultResults;

// If the default destination doesn't produce a constant result but is still
// reachable, and the lookup table has holes, we need to use a mask to
// determine if the current index should load from the lookup table or jump
// to the default case.
// The mask is unnecessary if the table has holes but the default destination
// is unreachable, as in that case the holes must also be unreachable.
bool NeedMask = AllHolesAreUndefined && DefaultIsReachable;
bool NeedMask = AllHolesArePoison && DefaultIsReachable;
if (NeedMask) {
// As an extra penalty for the validity test we require more cases.
if (SI->getNumCases() < 4) // FIXME: Find best threshold value (benchmark).
Expand Down Expand Up @@ -7157,9 +7170,11 @@ static bool switchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder,
for (PHINode *PHI : PHIs) {
const ResultListTy &ResultList = ResultLists[PHI];

Type *ResultType = ResultList.begin()->second->getType();

// Use any value to fill the lookup table holes.
Constant *DV =
AllHolesAreUndefined ? ResultLists[PHI][0].second : DefaultResults[PHI];
AllHolesArePoison ? PoisonValue::get(ResultType) : DefaultResults[PHI];
StringRef FuncName = Fn->getName();
SwitchLookupTable Table(Mod, TableSize, TableIndexOffset, ResultList, DV,
DL, FuncName);
Expand Down
232 changes: 229 additions & 3 deletions llvm/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,14 @@ target triple = "x86_64-unknown-linux-gnu"
; CHECK: @switch.table.unreachable_case = private unnamed_addr constant [9 x i32] [i32 0, i32 0, i32 0, i32 2, i32 -1, i32 1, i32 1, i32 1, i32 1], align 4
; CHECK: @switch.table.unreachable_default = private unnamed_addr constant [4 x i32] [i32 42, i32 52, i32 1, i32 2], align 4
; CHECK: @switch.table.nodefaultnoholes = private unnamed_addr constant [4 x i32] [i32 55, i32 123, i32 0, i32 -1], align 4
; CHECK: @switch.table.nodefaultwithholes = private unnamed_addr constant [6 x i32] [i32 55, i32 123, i32 0, i32 -1, i32 55, i32 -1], align 4
; CHECK: @switch.table.nodefaultwithholes = private unnamed_addr constant [6 x i32] [i32 55, i32 123, i32 0, i32 -1, i32 poison, i32 -1], align 4
; CHECK: @switch.table.threecases = private unnamed_addr constant [3 x i32] [i32 10, i32 7, i32 5], align 4
; CHECK: @switch.table.covered_switch_with_bit_tests = private unnamed_addr constant [8 x i32] [i32 2, i32 2, i32 2, i32 2, i32 2, i32 2, i32 1, i32 1], align 4
; CHECK: @switch.table.covered_switch_with_bit_tests = private unnamed_addr constant [8 x i32] [i32 2, i32 2, i32 poison, i32 poison, i32 poison, i32 poison, i32 1, i32 1], align 4
; CHECK: @switch.table.signed_overflow1 = private unnamed_addr constant [4 x i32] [i32 3333, i32 4444, i32 1111, i32 2222], align 4
; CHECK: @switch.table.signed_overflow2 = private unnamed_addr constant [4 x i32] [i32 3333, i32 4444, i32 2222, i32 2222], align 4
; CHECK: @switch.table.signed_overflow2 = private unnamed_addr constant [4 x i32] [i32 3333, i32 4444, i32 poison, i32 2222], align 4
; CHECK: @switch.table.constant_hole_unreachable_default_firstundef = private unnamed_addr constant [5 x i32] [i32 undef, i32 poison, i32 1, i32 1, i32 1], align 4
; CHECK: @switch.table.constant_hole_unreachable_default_lastundef = private unnamed_addr constant [5 x i32] [i32 1, i32 poison, i32 1, i32 1, i32 undef], align 4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dtcxzyw I don't know what impact this has on real-world code. Do you think it's worth running llvm-opt-benchmark?

; CHECK: @switch.table.linearmap_hole_unreachable_default = private unnamed_addr constant [5 x i32] [i32 1, i32 poison, i32 5, i32 7, i32 9], align 4
;.
define i32 @f(i32 %c) {
; CHECK-LABEL: @f(
Expand Down Expand Up @@ -2184,3 +2187,226 @@ return: ; preds = %sw.default, %entry,
%retval.0 = phi { i8, i8 } [ undef, %entry ], [ undef, %entry ], [ undef, %entry ], [ %1, %sw.default ]
ret { i8, i8 } %retval.0
}

; The switch has a hole which falls through to an unreachable default case, but it can still be optimized into a constant load because
; the poison value used for the hole is ignored.
define i32 @constant_hole_unreachable_default(i32 %x) {
; CHECK-LABEL: @constant_hole_unreachable_default(
; CHECK-NEXT: entry:
; CHECK-NEXT: ret i32 1
;
entry:
switch i32 %x, label %sw.default [
i32 0, label %bb0
i32 2, label %bb0
i32 3, label %bb0
i32 4, label %bb0
]

sw.default: unreachable
bb0: br label %return

return:
%res = phi i32 [ 1, %bb0 ]
ret i32 %res
}

; The switch has a hole which falls through to an unreachable default case and the first case explicitly returns undef, yet it cannot be optimized into a simple
; constant because we actually treat undef as a unique value rather than ignoring it.
define i32 @constant_hole_unreachable_default_firstundef(i32 %x) {
; CHECK-LABEL: @constant_hole_unreachable_default_firstundef(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[SWITCH_GEP:%.*]] = getelementptr inbounds [5 x i32], ptr @switch.table.constant_hole_unreachable_default_firstundef, i32 0, i32 [[X:%.*]]
; CHECK-NEXT: [[SWITCH_LOAD:%.*]] = load i32, ptr [[SWITCH_GEP]], align 4
; CHECK-NEXT: ret i32 [[SWITCH_LOAD]]
;
entry:
switch i32 %x, label %sw.default [
i32 0, label %bb.undef
i32 2, label %bb0
i32 3, label %bb0
i32 4, label %bb0
]

sw.default: unreachable
bb.undef: br label %return
bb0: br label %return

return:
%res = phi i32 [ undef, %bb.undef ], [ 1, %bb0 ]
ret i32 %res
}

; The switch has a hole which falls through to an unreachable default case and the last case explicitly returns undef, yet it cannot be optimized into a simple
; constant because we actually treat undef as a unique value rather than ignoring it.
define i32 @constant_hole_unreachable_default_lastundef(i32 %x) {
; CHECK-LABEL: @constant_hole_unreachable_default_lastundef(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[SWITCH_GEP:%.*]] = getelementptr inbounds [5 x i32], ptr @switch.table.constant_hole_unreachable_default_lastundef, i32 0, i32 [[X:%.*]]
; CHECK-NEXT: [[SWITCH_LOAD:%.*]] = load i32, ptr [[SWITCH_GEP]], align 4
; CHECK-NEXT: ret i32 [[SWITCH_LOAD]]
;
entry:
switch i32 %x, label %sw.default [
i32 0, label %bb0
i32 2, label %bb0
i32 3, label %bb0
i32 4, label %bb.undef
]

sw.default: unreachable
bb.undef: br label %return
bb0: br label %return

return:
%res = phi i32 [ undef, %bb.undef ], [ 1, %bb0 ]
ret i32 %res
}

; The switch has a hole which falls through to an unreachable default case and the first case explicitly returns poison, but it can still
; be optimized into a constant load because the poison values are ignored.
define i32 @constant_hole_unreachable_default_firstpoison(i32 %x) {
; CHECK-LABEL: @constant_hole_unreachable_default_firstpoison(
; CHECK-NEXT: entry:
; CHECK-NEXT: ret i32 1
;
entry:
switch i32 %x, label %sw.default [
i32 0, label %bb.poison
i32 2, label %bb0
i32 3, label %bb0
i32 4, label %bb0
]

sw.default: unreachable
bb.poison: br label %return
bb0: br label %return

return:
%res = phi i32 [ poison, %bb.poison ], [ 1, %bb0 ]
ret i32 %res
}

; The switch has a hole which falls through to an unreachable default case and the first case explicitly returns poison, but it can still
; be optimized into a constant load because the poison values are ignored.
define i32 @constant_hole_unreachable_default_lastpoison(i32 %x) {
; CHECK-LABEL: @constant_hole_unreachable_default_lastpoison(
; CHECK-NEXT: entry:
; CHECK-NEXT: ret i32 1
;
entry:
switch i32 %x, label %sw.default [
i32 0, label %bb0
i32 2, label %bb0
i32 3, label %bb0
i32 4, label %bb.poison
]

sw.default: unreachable
bb.poison: br label %return
bb0: br label %return

return:
%res = phi i32 [ poison, %bb.poison ], [ 1, %bb0 ]
ret i32 %res
}

define i32 @constant_hole_unreachable_default_undef_poison(i32 %x) {
; CHECK-LABEL: @constant_hole_unreachable_default_undef_poison(
; CHECK-NEXT: entry:
; CHECK-NEXT: ret i32 undef
;
entry:
switch i32 %x, label %sw.default [
i32 0, label %bb.undef
i32 2, label %bb.poison
i32 3, label %bb.poison
i32 4, label %bb.poison
]

sw.default: unreachable
bb.undef: br label %return
bb.poison: br label %return

return:
%res = phi i32 [ undef, %bb.undef ], [ poison, %bb.poison ]
ret i32 %res
}

define i32 @constant_hole_unreachable_default_poison_undef(i32 %x) {
; CHECK-LABEL: @constant_hole_unreachable_default_poison_undef(
; CHECK-NEXT: entry:
; CHECK-NEXT: ret i32 undef
;
entry:
switch i32 %x, label %sw.default [
i32 0, label %bb.poison
i32 2, label %bb.poison
i32 3, label %bb.poison
i32 4, label %bb.undef
]

sw.default: unreachable
bb.undef: br label %return
bb.poison: br label %return

return:
%res = phi i32 [ undef, %bb.undef ], [ poison, %bb.poison ]
ret i32 %res
}

; The switch has a hole which falls through to an unreachable default case, which prevents it from being optimized into a linear mapping 2*x+1.
; TODO: We should add support for this, at least in certain cases.
define i32 @linearmap_hole_unreachable_default(i32 %x) {
; CHECK-LABEL: @linearmap_hole_unreachable_default(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[SWITCH_GEP:%.*]] = getelementptr inbounds [5 x i32], ptr @switch.table.linearmap_hole_unreachable_default, i32 0, i32 [[X:%.*]]
; CHECK-NEXT: [[SWITCH_LOAD:%.*]] = load i32, ptr [[SWITCH_GEP]], align 4
; CHECK-NEXT: ret i32 [[SWITCH_LOAD]]
;
entry:
switch i32 %x, label %sw.default [
i32 0, label %bb0
i32 2, label %bb2
i32 3, label %bb3
i32 4, label %bb4
]

sw.default: unreachable
bb0: br label %return
bb2: br label %return
bb3: br label %return
bb4: br label %return

return:
%res = phi i32 [ 1, %bb0 ], [ 5, %bb2 ], [ 7, %bb3 ], [ 9, %bb4 ]
ret i32 %res
}

; The switch has a hole which falls through to an unreachable default case, but it can still be optimized into a bitmask extraction because
; the poison value used for the hole is simply replaced with zero.
define i1 @bitset_hole_unreachable_default(i32 %x) {
; CHECK-LABEL: @bitset_hole_unreachable_default(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[SWITCH_CAST:%.*]] = trunc i32 [[X:%.*]] to i5
; CHECK-NEXT: [[SWITCH_SHIFTAMT:%.*]] = mul nuw nsw i5 [[SWITCH_CAST]], 1
; CHECK-NEXT: [[SWITCH_DOWNSHIFT:%.*]] = lshr i5 8, [[SWITCH_SHIFTAMT]]
; CHECK-NEXT: [[SWITCH_MASKED:%.*]] = trunc i5 [[SWITCH_DOWNSHIFT]] to i1
; CHECK-NEXT: ret i1 [[SWITCH_MASKED]]
;
entry:
switch i32 %x, label %sw.default [
i32 0, label %bb0
i32 2, label %bb0
i32 3, label %bb1
i32 4, label %bb0
]

sw.default: unreachable
bb0: br label %return
bb1: br label %return

return:
%res = phi i1 [ 0, %bb0 ], [ 1, %bb1 ]
ret i1 %res
}
6 changes: 3 additions & 3 deletions llvm/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table_big.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ target triple = "i386-pc-linux-gnu"
;.
; CHECK: @switch.table.reachable_default_dense_0to31 = private unnamed_addr constant [32 x i32] [i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1], align 4
; CHECK: @switch.table.unreachable_default_dense_0to31 = private unnamed_addr constant [32 x i32] [i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1], align 4
; CHECK: @switch.table.reachable_default_holes_0to31 = private unnamed_addr constant [32 x i32] [i32 0, i32 7, i32 6, i32 0, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 0, i32 2, i32 1, i32 0, i32 7, i32 0, i32 5, i32 4, i32 3, i32 2, i32 0, i32 0, i32 7, i32 6, i32 5, i32 0, i32 3, i32 2, i32 1], align 4
; CHECK: @switch.table.unreachable_default_holes_0to31 = private unnamed_addr constant [32 x i32] [i32 0, i32 7, i32 6, i32 0, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 0, i32 2, i32 1, i32 0, i32 7, i32 0, i32 5, i32 4, i32 3, i32 2, i32 0, i32 0, i32 7, i32 6, i32 5, i32 0, i32 3, i32 2, i32 1], align 4
; CHECK: @switch.table.reachable_default_holes_0to31 = private unnamed_addr constant [32 x i32] [i32 0, i32 7, i32 6, i32 poison, i32 4, i32 3, i32 2, i32 1, i32 poison, i32 7, i32 6, i32 5, i32 4, i32 poison, i32 2, i32 1, i32 0, i32 7, i32 poison, i32 5, i32 4, i32 3, i32 2, i32 poison, i32 0, i32 7, i32 6, i32 5, i32 poison, i32 3, i32 2, i32 1], align 4
; CHECK: @switch.table.unreachable_default_holes_0to31 = private unnamed_addr constant [32 x i32] [i32 0, i32 7, i32 6, i32 poison, i32 4, i32 3, i32 2, i32 1, i32 poison, i32 7, i32 6, i32 5, i32 4, i32 poison, i32 2, i32 1, i32 0, i32 7, i32 poison, i32 5, i32 4, i32 3, i32 2, i32 poison, i32 0, i32 7, i32 6, i32 5, i32 poison, i32 3, i32 2, i32 1], align 4
; CHECK: @switch.table.reachable_default_dense_0to32 = private unnamed_addr constant [33 x i32] [i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0], align 4
; CHECK: @switch.table.unreachable_default_dense_0to32 = private unnamed_addr constant [33 x i32] [i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0], align 4
; CHECK: @switch.table.unreachable_default_holes_0to32 = private unnamed_addr constant [33 x i32] [i32 0, i32 7, i32 6, i32 0, i32 4, i32 3, i32 2, i32 1, i32 0, i32 7, i32 6, i32 5, i32 4, i32 0, i32 2, i32 1, i32 0, i32 7, i32 0, i32 5, i32 4, i32 3, i32 2, i32 0, i32 0, i32 7, i32 6, i32 5, i32 0, i32 3, i32 2, i32 1, i32 0], align 4
; CHECK: @switch.table.unreachable_default_holes_0to32 = private unnamed_addr constant [33 x i32] [i32 0, i32 7, i32 6, i32 poison, i32 4, i32 3, i32 2, i32 1, i32 poison, i32 7, i32 6, i32 5, i32 4, i32 poison, i32 2, i32 1, i32 0, i32 7, i32 poison, i32 5, i32 4, i32 3, i32 2, i32 poison, i32 0, i32 7, i32 6, i32 5, i32 poison, i32 3, i32 2, i32 1, i32 0], align 4
;.
define i32 @reachable_default_dense_0to31(i32 %x, i32 %y) {
; CHECK-LABEL: @reachable_default_dense_0to31(
Expand Down
Loading