Skip to content

Commit bec1279

Browse files
authored
Merge pull request #28719 from gottesmm/pr-39c568e8fa7a8e0f2ad1323b55401d557a150ad9
[diagnose-unreachable] Fix a small part of diagnose unreachable that did not handle ossa correctly.
2 parents 2a31c39 + b461eb0 commit bec1279

File tree

2 files changed

+105
-12
lines changed

2 files changed

+105
-12
lines changed

lib/SILOptimizer/Mandatory/DiagnoseUnreachable.cpp

Lines changed: 82 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,8 @@ static void propagateBasicBlockArgs(SILBasicBlock &BB) {
208208
static bool constantFoldEnumTerminator(SILBasicBlock &BB,
209209
UnreachableUserCodeReportingState *State,
210210
SwitchEnumInstBase *SUI,
211-
const EnumElementDecl *TheEnumElem,
212-
SILValue value = SILValue(),
213-
SILValue defaultValue = SILValue()) {
211+
EnumElementDecl *TheEnumElem,
212+
EnumInst *EnumInst) {
214213
SILBasicBlock *TheSuccessorBlock = nullptr;
215214
int ReachableBlockIdx = -1;
216215
for (unsigned Idx = 0; Idx < SUI->getNumCases(); ++Idx) {
@@ -251,11 +250,9 @@ static bool constantFoldEnumTerminator(SILBasicBlock &BB,
251250
// value.
252251
SILValue branchOperand;
253252
if (TheSuccessorBlock != DB) {
254-
assert(value);
255-
branchOperand = value;
253+
branchOperand = B.createUncheckedEnumData(Loc, EnumInst, TheEnumElem);
256254
} else {
257-
assert(defaultValue);
258-
branchOperand = defaultValue;
255+
branchOperand = EnumInst;
259256
}
260257
B.createBranch(Loc, TheSuccessorBlock, branchOperand);
261258
} else
@@ -297,6 +294,81 @@ static bool constantFoldEnumTerminator(SILBasicBlock &BB,
297294
return true;
298295
}
299296

297+
static bool constantFoldEnumAddrTerminator(
298+
SILBasicBlock &BB, UnreachableUserCodeReportingState *State,
299+
SwitchEnumInstBase *SUI, const EnumElementDecl *TheEnumElem) {
300+
SILBasicBlock *TheSuccessorBlock = nullptr;
301+
int ReachableBlockIdx = -1;
302+
for (unsigned Idx = 0; Idx < SUI->getNumCases(); ++Idx) {
303+
const EnumElementDecl *EI;
304+
SILBasicBlock *BI;
305+
std::tie(EI, BI) = SUI->getCase(Idx);
306+
if (EI == TheEnumElem) {
307+
TheSuccessorBlock = BI;
308+
ReachableBlockIdx = Idx;
309+
break;
310+
}
311+
}
312+
313+
SILBasicBlock *DB = nullptr;
314+
if (!TheSuccessorBlock) {
315+
if (SUI->hasDefault()) {
316+
DB = SUI->getDefaultBB();
317+
if (!isa<UnreachableInst>(DB->getTerminator())) {
318+
TheSuccessorBlock = DB;
319+
ReachableBlockIdx = SUI->getNumCases();
320+
}
321+
}
322+
}
323+
324+
// Not fully covered switches will be diagnosed later. SILGen represents
325+
// them with a Default basic block with an unreachable instruction.
326+
// We are going to produce an error on all unreachable instructions not
327+
// eliminated by DCE.
328+
if (!TheSuccessorBlock)
329+
return false;
330+
331+
// Replace the switch with a branch to the TheSuccessorBlock.
332+
SILBuilderWithScope B(&BB, SUI);
333+
SILLocation Loc = SUI->getLoc();
334+
B.createBranch(Loc, TheSuccessorBlock);
335+
336+
// Produce diagnostic info if we are not within an inlined function or
337+
// template instantiation.
338+
// FIXME: Do not report if we are within a template instantiation.
339+
assert(ReachableBlockIdx >= 0);
340+
if (Loc.is<RegularLocation>() && State) {
341+
// Find the first unreachable block in the switch so that we could use
342+
// it for better diagnostics.
343+
SILBasicBlock *UnreachableBlock = nullptr;
344+
if (SUI->getNumCases() > 1) {
345+
// More than one case.
346+
UnreachableBlock = (ReachableBlockIdx == 0) ? SUI->getCase(1).second
347+
: SUI->getCase(0).second;
348+
} else {
349+
if (SUI->getNumCases() == 1 && SUI->hasDefault()) {
350+
// One case and a default.
351+
UnreachableBlock = (ReachableBlockIdx == 0) ? SUI->getDefaultBB()
352+
: SUI->getCase(0).second;
353+
}
354+
}
355+
356+
// Generate diagnostic info.
357+
if (UnreachableBlock &&
358+
!State->PossiblyUnreachableBlocks.count(UnreachableBlock)) {
359+
State->PossiblyUnreachableBlocks.insert(UnreachableBlock);
360+
State->MetaMap.insert(std::pair<const SILBasicBlock *, UnreachableInfo>(
361+
UnreachableBlock,
362+
UnreachableInfo{UnreachableKind::FoldedSwitchEnum, Loc, true}));
363+
}
364+
}
365+
366+
LLVM_DEBUG(llvm::dbgs() << "Folding terminator: " << *SUI);
367+
recursivelyDeleteTriviallyDeadInstructions(SUI, true);
368+
NumTerminatorsFolded++;
369+
return true;
370+
}
371+
300372
static InjectEnumAddrInst *
301373
getAllocStackSingleInitializingInjectEnumAddr(SwitchEnumAddrInst *SEAI) {
302374
auto *stackSlot = dyn_cast<AllocStackInst>(SEAI->getOperand());
@@ -506,10 +578,8 @@ static bool constantFoldTerminator(SILBasicBlock &BB,
506578
// br bb2
507579
if (auto *SEI = dyn_cast<SwitchEnumInst>(TI)) {
508580
if (auto *TheEnum = dyn_cast<EnumInst>(SEI->getOperand())) {
509-
SILValue operand =
510-
TheEnum->hasOperand() ? TheEnum->getOperand() : SILValue();
511581
return constantFoldEnumTerminator(BB, State, SEI, TheEnum->getElement(),
512-
operand /*case*/, TheEnum /*default*/);
582+
TheEnum);
513583
}
514584
}
515585
if (auto *SEAI = dyn_cast<SwitchEnumAddrInst>(TI)) {
@@ -520,7 +590,8 @@ static bool constantFoldTerminator(SILBasicBlock &BB,
520590
//
521591
// TODO: This needs a better name.
522592
if (auto *IEAI = getAllocStackSingleInitializingInjectEnumAddr(SEAI)) {
523-
return constantFoldEnumTerminator(BB, State, SEAI, IEAI->getElement());
593+
return constantFoldEnumAddrTerminator(BB, State, SEAI,
594+
IEAI->getElement());
524595
}
525596
}
526597

test/SILOptimizer/diagnose_unreachable.sil

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,9 @@ class Klass2 {}
471471

472472
// CHECK-LABEL: sil [ossa] @constant_fold_diagnose_unreachable_succ_match : $@convention(thin) (@owned Klass1) -> () {
473473
// CHECK: bb0([[ARG:%.*]] :
474-
// CHECK: destroy_value [[ARG]]
474+
// CHECK: [[ENUM_VAL:%.*]] = enum $Either<Klass1, Klass2>, #Either.left!enumelt.1, [[ARG]]
475+
// CHECK: [[RESULT:%.*]] = unchecked_enum_data [[ENUM_VAL]]
476+
// CHECK: destroy_value [[RESULT]]
475477
// CHECK: } // end sil function 'constant_fold_diagnose_unreachable_succ_match'
476478
sil [ossa] @constant_fold_diagnose_unreachable_succ_match : $@convention(thin) (@owned Klass1) -> () {
477479
bb0(%0 : @owned $Klass1):
@@ -760,3 +762,23 @@ bb2:
760762
%9999 = tuple()
761763
return %9999 : $()
762764
}
765+
766+
// Just make sure we apply the optimization and do not trigger the ownership verifier
767+
//
768+
// CHECK-LABEL: sil [ossa] @constant_fold_switch_enum_with_owned_payload :
769+
// CHECK: bb0(
770+
// CHECK: br bb1
771+
// CHECK: bb1:
772+
// CHECK: return
773+
// CHECK: } // end sil function 'constant_fold_switch_enum_with_owned_payload'
774+
sil [ossa] @constant_fold_switch_enum_with_owned_payload : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject {
775+
bb0(%0 : @owned $Builtin.NativeObject):
776+
%1 = enum $FakeOptional<Builtin.NativeObject>, #FakeOptional.some!enumelt.1, %0 : $Builtin.NativeObject
777+
switch_enum %1 : $FakeOptional<Builtin.NativeObject>, case #FakeOptional.some!enumelt.1: bb1, case #FakeOptional.none!enumelt: bb2
778+
779+
bb1(%2 : @owned $Builtin.NativeObject):
780+
return %2 : $Builtin.NativeObject
781+
782+
bb2:
783+
unreachable
784+
}

0 commit comments

Comments
 (0)