Skip to content

Commit 2558230

Browse files
committed
Add a peephole to deal with enum regressions after changes to SILGen
Enum case constructors now directly store into the destination buffer without a copy. This prevents mem2reg on enums for loadable types. Localize the store to an intermediate buffer to enable mem2reg.
1 parent 81431b6 commit 2558230

File tree

2 files changed

+90
-6
lines changed

2 files changed

+90
-6
lines changed

lib/SILPasses/SILCombiner/SILCombinerMiscVisitors.cpp

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,8 @@ SILCombiner::visitInjectEnumAddrInst(InjectEnumAddrInst *IEAI) {
761761
SILBasicBlock::iterator II = IEAI->getIterator();
762762
StoreInst *SI = nullptr;
763763
InitEnumDataAddrInst *DataAddrInst = nullptr;
764+
ApplyInst *AI = nullptr;
765+
Operand *EnumInitOperand = nullptr;
764766
for (;;) {
765767
if (II == IEAI->getParent()->begin())
766768
return nullptr;
@@ -772,20 +774,76 @@ SILCombiner::visitInjectEnumAddrInst(InjectEnumAddrInst *IEAI) {
772774
DataAddrInst = dyn_cast<InitEnumDataAddrInst>(SI->getDest().getDef());
773775
if (DataAddrInst && DataAddrInst->getOperand() == IEAI->getOperand())
774776
break;
777+
SI = nullptr;
778+
}
779+
// Check whether we have an apply initializing the enum.
780+
// %iedai = init_enum_data_addr %enum_addr
781+
// = apply(%iedai,...)
782+
// inject_enum_addr %enum_addr
783+
//
784+
// We can localize the store to an alloc_stack.
785+
// Allowing us to perform the same optimization as for the store.
786+
//
787+
// %alloca = alloc_stack
788+
// apply(%alloca#1,...)
789+
// %load = load %alloca#1
790+
// %1 = enum $EnumType, $EnumType.case, %load
791+
// store %1 to %nopayload_addr
792+
//
793+
if ((AI = dyn_cast<ApplyInst>(&*II))) {
794+
auto Params = AI->getSubstCalleeType()->getParameters();
795+
unsigned ArgIdx = 0;
796+
for (auto &Opd : AI->getArgumentOperands()) {
797+
// Found an apply that initializes the enum. We can optimize this by
798+
// localizing the initialization to an alloc_stack and loading from it.
799+
DataAddrInst = dyn_cast<InitEnumDataAddrInst>(Opd.get().getDef());
800+
if (DataAddrInst && DataAddrInst->getOperand() == IEAI->getOperand() &&
801+
Params[ArgIdx].getConvention() ==
802+
ParameterConvention::Indirect_Out) {
803+
EnumInitOperand = &Opd;
804+
break;
805+
}
806+
++ArgIdx;
807+
}
808+
// We found an enum initialization.
809+
if (EnumInitOperand)
810+
break;
811+
AI = nullptr;
775812
}
776813
}
777814
// Found the store to this enum payload. Check if the store is the only use.
778815
if (!DataAddrInst->hasOneUse())
779816
return nullptr;
780817

781-
// In that case, create the payload enum/store.
782-
EnumInst *E =
818+
if (SI) {
819+
// In that case, create the payload enum/store.
820+
EnumInst *E =
783821
Builder.createEnum(DataAddrInst->getLoc(), SI->getSrc(),
784-
DataAddrInst->getElement(),
785-
DataAddrInst->getOperand().getType().getObjectType());
822+
DataAddrInst->getElement(),
823+
DataAddrInst->getOperand().getType().getObjectType());
824+
Builder.createStore(DataAddrInst->getLoc(), E, DataAddrInst->getOperand());
825+
// Cleanup.
826+
eraseInstFromFunction(*SI);
827+
eraseInstFromFunction(*DataAddrInst);
828+
return eraseInstFromFunction(*IEAI);
829+
}
830+
831+
assert(AI && "Must have an apply");
832+
// Localize the address access.
833+
Builder.setInsertionPoint(AI);
834+
auto *AllocStack = Builder.createAllocStack(DataAddrInst->getLoc(),
835+
EnumInitOperand->get().getType());
836+
EnumInitOperand->set(AllocStack->getAddressResult());
837+
Builder.setInsertionPoint(std::next(SILBasicBlock::iterator(AI)));
838+
SILValue Load(Builder.createLoad(DataAddrInst->getLoc(),
839+
AllocStack->getAddressResult()),
840+
0);
841+
EnumInst *E = Builder.createEnum(
842+
DataAddrInst->getLoc(), Load, DataAddrInst->getElement(),
843+
DataAddrInst->getOperand().getType().getObjectType());
786844
Builder.createStore(DataAddrInst->getLoc(), E, DataAddrInst->getOperand());
787-
// Cleanup.
788-
eraseInstFromFunction(*SI);
845+
Builder.createDeallocStack(DataAddrInst->getLoc(),
846+
AllocStack->getContainerResult());
789847
eraseInstFromFunction(*DataAddrInst);
790848
return eraseInstFromFunction(*IEAI);
791849
}

test/SILPasses/sil_combine.sil

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3033,3 +3033,29 @@ entry:
30333033
%r = load %p : $*Builtin.Int32
30343034
return %r : $Builtin.Int32
30353035
}
3036+
3037+
sil @init_enum : $@convention(thin) (@out B) -> ()
3038+
3039+
3040+
// Localize the initialization of the enum payload to enable mem2reg for the enum.
3041+
3042+
// CHECK-LABEL: sil @enum_promotion_case4
3043+
// CHECK: [[V1:%.*]] = alloc_stack $FakeOptional
3044+
// CHECK: [[V2:%.*]] = function_ref @init_enum
3045+
// CHECK: [[V3:%.*]] = alloc_stack $B
3046+
// CHECK: apply [[V2]]([[V3]]#1)
3047+
// CHECK: [[V4:%.*]] = load [[V3]]#1
3048+
// CHECK: [[V5:%.*]] = enum $FakeOptional<B>, #FakeOptional.Some!enumelt.1, [[V4]]
3049+
// CHECK: store [[V5]] to [[V1]]#1
3050+
3051+
sil @enum_promotion_case4 : $@convention(thin) () -> @owned FakeOptional<B> {
3052+
bb0:
3053+
%2 = alloc_stack $FakeOptional<B>
3054+
%3 = init_enum_data_addr %2#1 : $*FakeOptional<B>, #FakeOptional.Some!enumelt.1
3055+
%4 = function_ref @init_enum : $@convention(thin) (@out B) -> ()
3056+
%5 = apply %4(%3) : $@convention(thin) (@out B) -> ()
3057+
inject_enum_addr %2#1 : $*FakeOptional<B>, #FakeOptional.Some!enumelt.1
3058+
%7 = load %2#1 : $*FakeOptional<B>
3059+
dealloc_stack %2#0 : $*@local_storage FakeOptional<B>
3060+
return %7 : $FakeOptional<B>
3061+
}

0 commit comments

Comments
 (0)