Skip to content

Commit 1366f73

Browse files
nate-chandlernate-chandler
committed
[SILCombine] Dominance check for inject_enum_addr.
If stores of empty values to the address don't dominate the inject_enum_addr, the inject_enum_addr can't be eliminated--it's the only indication of the case that's in the address.
1 parent 6039a98 commit 1366f73

File tree

2 files changed

+270
-25
lines changed

2 files changed

+270
-25
lines changed

‎lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -865,14 +865,20 @@ SILInstruction *SILCombiner::visitCondFailInst(CondFailInst *CFI) {
865865
return nullptr;
866866
}
867867

868-
/// Create a value from stores to an address.
868+
/// Whether there exists a unique value to which \p addr is always initialized
869+
/// at \p forInst.
869870
///
870-
/// If there are only stores to \p addr, return the stored value. Also, if there
871-
/// are address projections, create aggregate instructions for it.
872-
/// If builder is null, it's just a dry-run to check if it's possible.
873-
static SILValue createValueFromAddr(SILValue addr, SILBuilder *builder,
874-
SILLocation loc) {
875-
SmallVector<SILValue, 4> elems;
871+
/// If \p builder is passed, create the value using it. If \p addr is
872+
/// initialized piecewise via initializations of tuple element memory, the full
873+
/// tuple is constructed via the builder.
874+
///
875+
/// A best effort.
876+
/// TODO: Construct structs.
877+
/// Handle stores of identical values on multiple paths.
878+
static std::optional<std::pair<SILValue, SILInstruction *>>
879+
createValueFromAddr(SILValue addr, SILInstruction *forInst, DominanceInfo *DI,
880+
SILBuilder *builder, SILLocation loc) {
881+
SmallVector<std::optional<std::pair<SILValue, SILInstruction *>>, 4> pairs;
876882
enum Kind {
877883
none, store, tuple
878884
} kind = none;
@@ -884,7 +890,7 @@ static SILValue createValueFromAddr(SILValue addr, SILBuilder *builder,
884890

885891
auto *st = dyn_cast<StoreInst>(user);
886892
if (st && kind == none && st->getDest() == addr) {
887-
elems.push_back(st->getSrc());
893+
pairs.push_back({{st->getSrc(), st}});
888894
kind = store;
889895
// We cannot just return st->getSrc() here because we also have to check
890896
// if the store destination is the only use of addr.
@@ -893,35 +899,53 @@ static SILValue createValueFromAddr(SILValue addr, SILBuilder *builder,
893899

894900
if (auto *telem = dyn_cast<TupleElementAddrInst>(user)) {
895901
if (kind == none) {
896-
elems.resize(addr->getType().castTo<TupleType>()->getNumElements());
902+
pairs.resize(addr->getType().castTo<TupleType>()->getNumElements());
897903
kind = tuple;
898904
}
899905
if (kind == tuple) {
900-
if (elems[telem->getFieldIndex()])
901-
return SILValue();
902-
elems[telem->getFieldIndex()] = createValueFromAddr(telem, builder, loc);
906+
if (pairs[telem->getFieldIndex()]) {
907+
// Already found a tuple_element_addr at this index. Assume that a
908+
// different value is stored to addr by it.
909+
return std::nullopt;
910+
}
911+
pairs[telem->getFieldIndex()] =
912+
createValueFromAddr(telem, forInst, DI, builder, loc);
903913
continue;
904914
}
905915
}
906916
// TODO: handle StructElementAddrInst to create structs.
907917

908-
return SILValue();
918+
return std::nullopt;
909919
}
910920
switch (kind) {
911921
case none:
912-
return SILValue();
922+
return std::nullopt;
913923
case store:
914-
assert(elems.size() == 1);
915-
return elems[0];
924+
assert(pairs.size() == 1);
925+
{
926+
auto pair = pairs[0];
927+
assert(pair.has_value());
928+
if (!DI->properlyDominates(pair->second, forInst))
929+
return std::nullopt;
930+
return pair;
931+
}
916932
case tuple:
917-
if (std::any_of(elems.begin(), elems.end(),
918-
[](SILValue v){ return !(bool)v; }))
919-
return SILValue();
933+
if (std::any_of(pairs.begin(), pairs.end(), [&](auto pair) {
934+
return !pair.has_value() ||
935+
!DI->properlyDominates(pair->second, forInst);
936+
}))
937+
return std::nullopt;
920938
if (builder) {
921-
return builder->createTuple(loc, addr->getType().getObjectType(), elems);
939+
SmallVector<SILValue, 4> elements;
940+
for (auto pair : pairs) {
941+
elements.push_back(pair->first);
942+
}
943+
auto *tuple =
944+
builder->createTuple(loc, addr->getType().getObjectType(), elements);
945+
return {{tuple, tuple}};
922946
}
923947
// Just return anything not null for the dry-run.
924-
return elems[0];
948+
return pairs[0];
925949
}
926950
llvm_unreachable("invalid kind");
927951
}
@@ -1185,9 +1209,12 @@ SILCombiner::visitInjectEnumAddrInst(InjectEnumAddrInst *IEAI) {
11851209
// store %payload1 to %elem1_addr
11861210
// inject_enum_addr %enum_addr, $EnumType.case
11871211
//
1188-
if (createValueFromAddr(DataAddrInst, nullptr, DataAddrInst->getLoc())) {
1189-
SILValue en =
1190-
createValueFromAddr(DataAddrInst, &Builder, DataAddrInst->getLoc());
1212+
auto DI = DA->get(IEAI->getFunction());
1213+
if (createValueFromAddr(DataAddrInst, IEAI, DI, nullptr,
1214+
DataAddrInst->getLoc())) {
1215+
SILValue en = createValueFromAddr(DataAddrInst, IEAI, DI, &Builder,
1216+
DataAddrInst->getLoc())
1217+
->first;
11911218
assert(en);
11921219

11931220
// In that case, create the payload enum/store.
@@ -1219,7 +1246,7 @@ SILCombiner::visitInjectEnumAddrInst(InjectEnumAddrInst *IEAI) {
12191246
// store %1 to %nopayload_addr
12201247
//
12211248
auto *AI = dyn_cast_or_null<ApplyInst>(getSingleNonDebugUser(DataAddrInst));
1222-
if (!AI)
1249+
if (!AI || !DI->properlyDominates(AI, IEAI))
12231250
return nullptr;
12241251

12251252
unsigned ArgIdx = 0;

‎test/SILOptimizer/sil_combine_enum_addr.sil

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,3 +392,221 @@ bb3:
392392
%8 = tuple ()
393393
return %8 : $()
394394
}
395+
396+
sil @getVoidOut : $@convention(thin) () -> @out ()
397+
398+
// CHECK-LABEL: sil @empty_nondominating_apply : {{.*}} {
399+
// CHECK: inject_enum_addr
400+
// CHECK-LABEL: } // end sil function 'empty_nondominating_apply'
401+
sil @empty_nondominating_apply : $@convention(thin) (Builtin.Int1) -> () {
402+
entry(%cond : $Builtin.Int1):
403+
%addr = alloc_stack $Optional<()>
404+
%some_addr = init_enum_data_addr %addr : $*Optional<()>, #Optional.some!enumelt
405+
cond_br %cond, left, right
406+
407+
left:
408+
br middle
409+
410+
right:
411+
%get = function_ref @getVoidOut : $@convention(thin) () -> @out ()
412+
apply %get(%some_addr) : $@convention(thin) () -> @out ()
413+
br middle
414+
415+
middle:
416+
inject_enum_addr %addr : $*Optional<()>, #Optional.some!enumelt
417+
%o = load %addr : $*Optional<()>
418+
switch_enum %o : $Optional<()>, case #Optional.some!enumelt: some_block, case #Optional.none!enumelt: none_block
419+
420+
some_block(%avoid : $()):
421+
br exit
422+
423+
none_block:
424+
br exit
425+
426+
exit:
427+
dealloc_stack %addr : $*Optional<()>
428+
%retval = tuple ()
429+
return %retval : $()
430+
}
431+
432+
// CHECK-LABEL: sil @empty_nondominating_store : {{.*}} {
433+
// CHECK: inject_enum_addr
434+
// CHECK-LABEL: } // end sil function 'empty_nondominating_store'
435+
sil @empty_nondominating_store : $@convention(thin) (Builtin.Int1) -> () {
436+
entry(%cond : $Builtin.Int1):
437+
%addr = alloc_stack $Optional<()>
438+
%some_addr = init_enum_data_addr %addr : $*Optional<()>, #Optional.some!enumelt
439+
cond_br %cond, left, right
440+
441+
left:
442+
br middle
443+
444+
right:
445+
%void = tuple ()
446+
store %void to %some_addr : $*()
447+
br middle
448+
449+
middle:
450+
inject_enum_addr %addr : $*Optional<()>, #Optional.some!enumelt
451+
%o = load %addr : $*Optional<()>
452+
switch_enum %o : $Optional<()>, case #Optional.some!enumelt: some_block, case #Optional.none!enumelt: none_block
453+
454+
some_block(%avoid : $()):
455+
br exit
456+
457+
none_block:
458+
br exit
459+
460+
exit:
461+
dealloc_stack %addr : $*Optional<()>
462+
%retval = tuple ()
463+
return %retval : $()
464+
}
465+
466+
// CHECK-LABEL: sil @empty_nondominating_tuple_store_neither : {{.*}} {
467+
// CHECK: inject_enum_addr
468+
// CHECK-LABEL: } // end sil function 'empty_nondominating_tuple_store_neither'
469+
sil @empty_nondominating_tuple_store_neither : $@convention(thin) (Builtin.Int1) -> () {
470+
entry(%cond : $Builtin.Int1):
471+
%addr = alloc_stack $Optional<((), ())>
472+
%some_addr = init_enum_data_addr %addr : $*Optional<((), ())>, #Optional.some!enumelt
473+
%left_addr = tuple_element_addr %some_addr : $*((), ()), 0
474+
%right_addr = tuple_element_addr %some_addr : $*((), ()), 1
475+
cond_br %cond, left, right
476+
477+
left:
478+
br middle
479+
480+
right:
481+
%void = tuple ()
482+
store %void to %left_addr : $*()
483+
store %void to %right_addr : $*()
484+
br middle
485+
486+
middle:
487+
inject_enum_addr %addr : $*Optional<((), ())>, #Optional.some!enumelt
488+
%o = load %addr : $*Optional<((), ())>
489+
switch_enum %o : $Optional<((), ())>, case #Optional.some!enumelt: some_block, case #Optional.none!enumelt: none_block
490+
491+
some_block(%avoid : $((), ())):
492+
br exit
493+
494+
none_block:
495+
br exit
496+
497+
exit:
498+
dealloc_stack %addr : $*Optional<((), ())>
499+
%retval = tuple ()
500+
return %retval : $()
501+
}
502+
503+
// CHECK-LABEL: sil @empty_nondominating_tuple_store_left : {{.*}} {
504+
// CHECK: inject_enum_addr
505+
// CHECK-LABEL: } // end sil function 'empty_nondominating_tuple_store_left'
506+
sil @empty_nondominating_tuple_store_left : $@convention(thin) (Builtin.Int1) -> () {
507+
entry(%cond : $Builtin.Int1):
508+
%addr = alloc_stack $Optional<((), ())>
509+
%some_addr = init_enum_data_addr %addr : $*Optional<((), ())>, #Optional.some!enumelt
510+
%left_addr = tuple_element_addr %some_addr : $*((), ()), 0
511+
%right_addr = tuple_element_addr %some_addr : $*((), ()), 1
512+
%void = tuple ()
513+
cond_br %cond, left, right
514+
515+
left:
516+
br middle
517+
518+
right:
519+
store %void to %left_addr : $*()
520+
br middle
521+
522+
middle:
523+
store %void to %right_addr : $*()
524+
inject_enum_addr %addr : $*Optional<((), ())>, #Optional.some!enumelt
525+
%o = load %addr : $*Optional<((), ())>
526+
switch_enum %o : $Optional<((), ())>, case #Optional.some!enumelt: some_block, case #Optional.none!enumelt: none_block
527+
528+
some_block(%avoid : $((), ())):
529+
br exit
530+
531+
none_block:
532+
br exit
533+
534+
exit:
535+
dealloc_stack %addr : $*Optional<((), ())>
536+
%retval = tuple ()
537+
return %retval : $()
538+
}
539+
540+
// CHECK-LABEL: sil @empty_nondominating_tuple_store_right : {{.*}} {
541+
// CHECK: inject_enum_addr
542+
// CHECK-LABEL: } // end sil function 'empty_nondominating_tuple_store_right'
543+
sil @empty_nondominating_tuple_store_right : $@convention(thin) (Builtin.Int1) -> () {
544+
entry(%cond : $Builtin.Int1):
545+
%addr = alloc_stack $Optional<((), ())>
546+
%some_addr = init_enum_data_addr %addr : $*Optional<((), ())>, #Optional.some!enumelt
547+
%left_addr = tuple_element_addr %some_addr : $*((), ()), 0
548+
%right_addr = tuple_element_addr %some_addr : $*((), ()), 1
549+
%void = tuple ()
550+
cond_br %cond, left, right
551+
552+
left:
553+
br middle
554+
555+
right:
556+
store %void to %right_addr : $*()
557+
br middle
558+
559+
middle:
560+
store %void to %left_addr : $*()
561+
inject_enum_addr %addr : $*Optional<((), ())>, #Optional.some!enumelt
562+
%o = load %addr : $*Optional<((), ())>
563+
switch_enum %o : $Optional<((), ())>, case #Optional.some!enumelt: some_block, case #Optional.none!enumelt: none_block
564+
565+
some_block(%avoid : $((), ())):
566+
br exit
567+
568+
none_block:
569+
br exit
570+
571+
exit:
572+
dealloc_stack %addr : $*Optional<((), ())>
573+
%retval = tuple ()
574+
return %retval : $()
575+
}
576+
577+
// CHECK-LABEL: sil @empty_dominating_tuple_stores : {{.*}} {
578+
// CHECK-NOT: inject_enum_addr
579+
// CHECK-LABEL: } // end sil function 'empty_dominating_tuple_stores'
580+
sil @empty_dominating_tuple_stores : $@convention(thin) (Builtin.Int1) -> () {
581+
entry(%cond : $Builtin.Int1):
582+
%addr = alloc_stack $Optional<((), ())>
583+
%some_addr = init_enum_data_addr %addr : $*Optional<((), ())>, #Optional.some!enumelt
584+
%left_addr = tuple_element_addr %some_addr : $*((), ()), 0
585+
%right_addr = tuple_element_addr %some_addr : $*((), ()), 1
586+
%void = tuple ()
587+
cond_br %cond, left, right
588+
589+
left:
590+
br middle
591+
592+
right:
593+
br middle
594+
595+
middle:
596+
store %void to %right_addr : $*()
597+
store %void to %left_addr : $*()
598+
inject_enum_addr %addr : $*Optional<((), ())>, #Optional.some!enumelt
599+
%o = load %addr : $*Optional<((), ())>
600+
switch_enum %o : $Optional<((), ())>, case #Optional.some!enumelt: some_block, case #Optional.none!enumelt: none_block
601+
602+
some_block(%avoid : $((), ())):
603+
br exit
604+
605+
none_block:
606+
br exit
607+
608+
exit:
609+
dealloc_stack %addr : $*Optional<((), ())>
610+
%retval = tuple ()
611+
return %retval : $()
612+
}

0 commit comments

Comments
 (0)