Skip to content

Commit a5e560f

Browse files
author
Joe Shajrawi
committed
[SILOptimizer] simplify enum frontend pattern, add support for cross-basic block enums - rdar://problem/27640524
1 parent b120535 commit a5e560f

File tree

2 files changed

+224
-55
lines changed

2 files changed

+224
-55
lines changed

‎lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

Lines changed: 137 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,22 @@
1212

1313
#define DEBUG_TYPE "sil-combine"
1414
#include "SILCombiner.h"
15+
#include "swift/Basic/STLExtras.h"
16+
#include "swift/SIL/DebugUtils.h"
1517
#include "swift/SIL/DynamicCasts.h"
1618
#include "swift/SIL/PatternMatch.h"
1719
#include "swift/SIL/Projection.h"
1820
#include "swift/SIL/SILBuilder.h"
1921
#include "swift/SIL/SILVisitor.h"
20-
#include "swift/SIL/DebugUtils.h"
21-
#include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
2222
#include "swift/SILOptimizer/Analysis/ARCAnalysis.h"
23+
#include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
2324
#include "swift/SILOptimizer/Analysis/CFG.h"
2425
#include "swift/SILOptimizer/Analysis/ValueTracking.h"
25-
#include "swift/SILOptimizer/Utils/Local.h"
2626
#include "swift/SILOptimizer/Utils/Devirtualize.h"
27+
#include "swift/SILOptimizer/Utils/Local.h"
28+
#include "llvm/ADT/DenseMap.h"
2729
#include "llvm/ADT/SmallPtrSet.h"
2830
#include "llvm/ADT/SmallVector.h"
29-
#include "llvm/ADT/DenseMap.h"
3031

3132
using namespace swift;
3233
using namespace swift::PatternMatch;
@@ -781,75 +782,156 @@ SILCombiner::visitInjectEnumAddrInst(InjectEnumAddrInst *IEAI) {
781782

782783
// Ok, we have a payload enum, make sure that we have a store previous to
783784
// us...
784-
SILBasicBlock::iterator II = IEAI->getIterator();
785-
StoreInst *SI = nullptr;
785+
SILValue ASO = IEAI->getOperand();
786+
if (!isa<AllocStackInst>(ASO)) {
787+
return nullptr;
788+
}
786789
InitEnumDataAddrInst *DataAddrInst = nullptr;
787-
ApplyInst *AI = nullptr;
788-
Operand *EnumInitOperand = nullptr;
789-
for (;;) {
790-
if (II == IEAI->getParent()->begin())
791-
return nullptr;
792-
--II;
793-
SI = dyn_cast<StoreInst>(&*II);
794-
if (SI) {
795-
// Find a Store whose destination is taken from an init_enum_data_addr
796-
// whose address is same allocation as our inject_enum_addr.
797-
DataAddrInst = dyn_cast<InitEnumDataAddrInst>(SI->getDest());
798-
if (DataAddrInst && DataAddrInst->getOperand() == IEAI->getOperand())
799-
break;
800-
SI = nullptr;
790+
InjectEnumAddrInst *EnumAddrIns = nullptr;
791+
llvm::SmallPtrSet<SILInstruction *, 32> WriteSet;
792+
for (auto UsersIt : ASO->getUses()) {
793+
SILInstruction *CurrUser = UsersIt->getUser();
794+
if (CurrUser->isDeallocatingStack()) {
795+
// we don't care about the dealloc stack instructions
796+
continue;
801797
}
802-
// Check whether we have an apply initializing the enum.
803-
// %iedai = init_enum_data_addr %enum_addr
804-
// = apply(%iedai,...)
805-
// inject_enum_addr %enum_addr
806-
//
807-
// We can localize the store to an alloc_stack.
808-
// Allowing us to perform the same optimization as for the store.
809-
//
810-
// %alloca = alloc_stack
811-
// apply(%alloca,...)
812-
// %load = load %alloca
813-
// %1 = enum $EnumType, $EnumType.case, %load
814-
// store %1 to %nopayload_addr
815-
//
816-
if ((AI = dyn_cast<ApplyInst>(&*II))) {
817-
unsigned ArgIdx = 0;
818-
for (auto &Opd : AI->getArgumentOperands()) {
819-
// Found an apply that initializes the enum. We can optimize this by
820-
// localizing the initialization to an alloc_stack and loading from it.
821-
DataAddrInst = dyn_cast<InitEnumDataAddrInst>(Opd.get());
822-
if (DataAddrInst && DataAddrInst->getOperand() == IEAI->getOperand() &&
823-
ArgIdx < AI->getSubstCalleeType()->getNumIndirectResults()) {
824-
EnumInitOperand = &Opd;
798+
if (isDebugInst(CurrUser) || isa<LoadInst>(CurrUser)) {
799+
// These Instructions are a non-risky use we can ignore
800+
continue;
801+
}
802+
if (auto *CurrInst = dyn_cast<InitEnumDataAddrInst>(CurrUser)) {
803+
if (DataAddrInst) {
804+
return nullptr;
805+
}
806+
DataAddrInst = CurrInst;
807+
continue;
808+
}
809+
if (auto *CurrInst = dyn_cast<InjectEnumAddrInst>(CurrUser)) {
810+
if (EnumAddrIns) {
811+
return nullptr;
812+
}
813+
EnumAddrIns = CurrInst;
814+
continue;
815+
}
816+
if (isa<StoreInst>(CurrUser)) {
817+
// The only MayWrite Instruction we can safely handle
818+
WriteSet.insert(CurrUser);
819+
continue;
820+
}
821+
// It is too risky to continue if it is any other instruction.
822+
return nullptr;
823+
}
824+
825+
if (!DataAddrInst || !EnumAddrIns) {
826+
return nullptr;
827+
}
828+
assert((EnumAddrIns == IEAI) &&
829+
"Found InitEnumDataAddrInst differs from IEAI");
830+
// Found the DataAddrInst to this enum payload. Check if it has only use.
831+
if (!hasOneNonDebugUse(DataAddrInst))
832+
return nullptr;
833+
834+
StoreInst *SI = dyn_cast<StoreInst>(getSingleNonDebugUser(DataAddrInst));
835+
ApplyInst *AI = dyn_cast<ApplyInst>(getSingleNonDebugUser(DataAddrInst));
836+
if (!SI && !AI) {
837+
return nullptr;
838+
}
839+
840+
// Make sure the enum pattern instructions are the only ones which write to
841+
// this location
842+
if (!WriteSet.empty()) {
843+
// Analyze the instructions (implicit dominator analysis)
844+
// If we find any of MayWriteSet, return nullptr
845+
SILBasicBlock *InitEnumBB = DataAddrInst->getParent();
846+
assert(InitEnumBB && "DataAddrInst is not in a valid Basic Block");
847+
llvm::SmallVector<SILInstruction *, 64> Worklist;
848+
Worklist.push_back(IEAI);
849+
llvm::SmallPtrSet<SILBasicBlock *, 16> Preds;
850+
Preds.insert(IEAI->getParent());
851+
while (!Worklist.empty()) {
852+
SILInstruction *CurrIns = Worklist.pop_back_val();
853+
SILBasicBlock *CurrBB = CurrIns->getParent();
854+
855+
if (CurrBB->isEntry() && CurrBB != InitEnumBB) {
856+
// reached prologue without encountering the init bb
857+
return nullptr;
858+
}
859+
860+
for (llvm::iplist<SILInstruction>::reverse_iterator InsIt(
861+
CurrIns->getIterator());
862+
InsIt != CurrBB->rend(); ++InsIt) {
863+
SILInstruction *Ins = &*InsIt;
864+
if (Ins == DataAddrInst) {
865+
// don't care about what comes before init enum in the basic block
825866
break;
826867
}
827-
++ArgIdx;
868+
if (WriteSet.count(Ins) != 0) {
869+
return nullptr;
870+
}
871+
}
872+
873+
if (CurrBB == InitEnumBB) {
874+
continue;
875+
}
876+
877+
// Go to predecessors and do all that again
878+
for (SILBasicBlock *Pred : CurrBB->getPreds()) {
879+
// If it's already in the set, then we've already queued and/or
880+
// processed the predecessors.
881+
if (Preds.insert(Pred).second) {
882+
Worklist.push_back(&*Pred->rbegin());
883+
}
828884
}
829-
// We found an enum initialization.
830-
if (EnumInitOperand)
831-
break;
832-
AI = nullptr;
833885
}
834886
}
835-
// Found the store to this enum payload. Check if the store is the only use.
836-
if (!DataAddrInst->hasOneUse())
837-
return nullptr;
838887

839888
if (SI) {
889+
assert((SI->getDest() == DataAddrInst) &&
890+
"Can't find StoreInst with DataAddrInst as its destination");
840891
// In that case, create the payload enum/store.
841-
EnumInst *E =
842-
Builder.createEnum(DataAddrInst->getLoc(), SI->getSrc(),
843-
DataAddrInst->getElement(),
844-
DataAddrInst->getOperand()->getType().getObjectType());
892+
EnumInst *E = Builder.createEnum(
893+
DataAddrInst->getLoc(), SI->getSrc(), DataAddrInst->getElement(),
894+
DataAddrInst->getOperand()->getType().getObjectType());
845895
Builder.createStore(DataAddrInst->getLoc(), E, DataAddrInst->getOperand());
846896
// Cleanup.
847897
eraseInstFromFunction(*SI);
848898
eraseInstFromFunction(*DataAddrInst);
849899
return eraseInstFromFunction(*IEAI);
850900
}
851901

902+
// Check whether we have an apply initializing the enum.
903+
// %iedai = init_enum_data_addr %enum_addr
904+
// = apply(%iedai,...)
905+
// inject_enum_addr %enum_addr
906+
//
907+
// We can localize the store to an alloc_stack.
908+
// Allowing us to perform the same optimization as for the store.
909+
//
910+
// %alloca = alloc_stack
911+
// apply(%alloca,...)
912+
// %load = load %alloca
913+
// %1 = enum $EnumType, $EnumType.case, %load
914+
// store %1 to %nopayload_addr
915+
//
852916
assert(AI && "Must have an apply");
917+
unsigned ArgIdx = 0;
918+
Operand *EnumInitOperand = nullptr;
919+
for (auto &Opd : AI->getArgumentOperands()) {
920+
// Found an apply that initializes the enum. We can optimize this by
921+
// localizing the initialization to an alloc_stack and loading from it.
922+
DataAddrInst = dyn_cast<InitEnumDataAddrInst>(Opd.get());
923+
if (DataAddrInst && DataAddrInst->getOperand() == IEAI->getOperand() &&
924+
ArgIdx < AI->getSubstCalleeType()->getNumIndirectResults()) {
925+
EnumInitOperand = &Opd;
926+
break;
927+
}
928+
++ArgIdx;
929+
}
930+
931+
if (!EnumInitOperand) {
932+
return nullptr;
933+
}
934+
853935
// Localize the address access.
854936
Builder.setInsertionPoint(AI);
855937
auto *AllocStack = Builder.createAllocStack(DataAddrInst->getLoc(),

‎test/SILOptimizer/sil_combine_enums.sil

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,93 @@ bb0(%0 : $*Int32, %1 : $Builtin.Int32):
157157
return %i : $Int32
158158
}
159159

160+
// CHECK-LABEL: sil @canonicalize_init_enum_data_addr_diff_basic_blocks
161+
// CHECK-NOT: init_enum_data_addr
162+
// CHECK-NOT: inject_enum_addr
163+
// CHECK: enum $Optional<Int32>, #Optional.some!enumelt.1
164+
// CHECK-NOT: inject_enum_addr
165+
// CHECK: return
166+
sil @canonicalize_init_enum_data_addr_diff_basic_blocks : $@convention(thin) (@inout Int32, Builtin.Int32) -> Int32 {
167+
bb0(%0 : $*Int32, %1 : $Builtin.Int32):
168+
%s1 = alloc_stack $Optional<Int32>
169+
%e1 = init_enum_data_addr %s1 : $*Optional<Int32>, #Optional.some!enumelt.1
170+
%v = load %0 : $*Int32
171+
store %v to %e1 : $*Int32
172+
%i1 = integer_literal $Builtin.Int32, 1
173+
%i0 = integer_literal $Builtin.Int1, 0
174+
%a = builtin "sadd_with_overflow_Int32"(%1 : $Builtin.Int32, %i1 : $Builtin.Int32, %i0 : $Builtin.Int1) : $(Builtin.Int32, Builtin.Int1)
175+
%w = tuple_extract %a : $(Builtin.Int32, Builtin.Int1), 0
176+
%i = struct $Int32 (%w : $Builtin.Int32)
177+
br bb1
178+
179+
bb1: // Preds: bb0
180+
store %i to %0 : $*Int32
181+
inject_enum_addr %s1 : $*Optional<Int32>, #Optional.some!enumelt.1
182+
dealloc_stack %s1 : $*Optional<Int32>
183+
return %i : $Int32
184+
}
185+
186+
// CHECK-LABEL: sil @fail_to_canonicalize_init_enum_data_addr_reach_entry
187+
// CHECK: init_enum_data_addr
188+
// CHECK: inject_enum_addr
189+
// CHECK-NOT: enum $Optional<Int32>, #Optional.some!enumelt.1
190+
// CHECK: return
191+
sil @fail_to_canonicalize_init_enum_data_addr_reach_entry : $@convention(thin) (@inout Int32, Builtin.Int32, Builtin.Int1) -> Int32 {
192+
bb0(%0 : $*Int32, %1 : $Builtin.Int32, %2 : $Builtin.Int1):
193+
%s1 = alloc_stack $Optional<Int32>
194+
%i2 = load %0 : $*Int32
195+
%en = enum $Optional<Int32>, #Optional.none!enumelt
196+
store %en to %s1 : $*Optional<Int32>
197+
cond_br %2, bb1, bb2
198+
199+
bb1:
200+
%e1 = init_enum_data_addr %s1 : $*Optional<Int32>, #Optional.some!enumelt.1
201+
%v = load %0 : $*Int32
202+
store %v to %e1 : $*Int32
203+
%i1 = integer_literal $Builtin.Int32, 1
204+
%i0 = integer_literal $Builtin.Int1, 0
205+
%a = builtin "sadd_with_overflow_Int32"(%1 : $Builtin.Int32, %i1 : $Builtin.Int32, %i0 : $Builtin.Int1) : $(Builtin.Int32, Builtin.Int1)
206+
%w = tuple_extract %a : $(Builtin.Int32, Builtin.Int1), 0
207+
%i = struct $Int32 (%w : $Builtin.Int32)
208+
store %i to %0 : $*Int32
209+
br bb2
210+
211+
bb2: // Preds: bb0 bb1
212+
inject_enum_addr %s1 : $*Optional<Int32>, #Optional.some!enumelt.1
213+
dealloc_stack %s1 : $*Optional<Int32>
214+
return %i2 : $Int32
215+
}
216+
217+
// CHECK-LABEL: sil @fail_to_canonicalize_init_enum_data_addr
218+
// CHECK: init_enum_data_addr
219+
// CHECK: inject_enum_addr
220+
// CHECK-NOT: enum $Optional<Int32>, #Optional.some!enumelt.1
221+
// CHECK: return
222+
sil @fail_to_canonicalize_init_enum_data_addr : $@convention(thin) (@inout Int32, Builtin.Int32) -> Int32 {
223+
bb0(%0 : $*Int32, %1 : $Builtin.Int32):
224+
%s1 = alloc_stack $Optional<Int32>
225+
%e1 = init_enum_data_addr %s1 : $*Optional<Int32>, #Optional.some!enumelt.1
226+
%v = load %0 : $*Int32
227+
store %v to %e1 : $*Int32
228+
%i1 = integer_literal $Builtin.Int32, 1
229+
%i0 = integer_literal $Builtin.Int1, 0
230+
%a = builtin "sadd_with_overflow_Int32"(%1 : $Builtin.Int32, %i1 : $Builtin.Int32, %i0 : $Builtin.Int1) : $(Builtin.Int32, Builtin.Int1)
231+
%w = tuple_extract %a : $(Builtin.Int32, Builtin.Int1), 0
232+
%i = struct $Int32 (%w : $Builtin.Int32)
233+
br bb1
234+
235+
bb1:
236+
%o = enum $Optional<Int32>, #Optional.none!enumelt
237+
store %o to %s1 : $*Optional<Int32>
238+
br bb2
239+
240+
bb2:
241+
store %i to %0 : $*Int32
242+
inject_enum_addr %s1 : $*Optional<Int32>, #Optional.some!enumelt.1
243+
dealloc_stack %s1 : $*Optional<Int32>
244+
return %i : $Int32
245+
}
246+
160247
// Check the cond_br(select_enum) -> switch_enum conversion.
161248
//
162249
// CHECK-LABEL: sil @convert_select_enum_cond_br_to_switch_enum

0 commit comments

Comments
 (0)