Skip to content

Commit e8a6646

Browse files
gottesmmgottesmm
authored
Merge pull request #21791 from gottesmm/pr-5a4e17411328a11ec4d37cc30ead812b7087badf
[silcombine] (store (struct_gep addr) value) -> (store addr (struct v…
2 parents 908646f + cf43a93 commit e8a6646

File tree

5 files changed

+200
-60
lines changed

5 files changed

+200
-60
lines changed

‎lib/SILOptimizer/IPO/GlobalOpt.cpp

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,6 @@ class SILGlobalOpt {
137137
/// can be statically initialized.
138138
void optimizeInitializer(SILFunction *AddrF, GlobalInitCalls &Calls);
139139

140-
/// Perform peephole optimizations on the initializer list.
141-
void peepholeInitializer(SILFunction *InitFunc);
142-
143140
/// Optimize access to the global variable, which is known to have a constant
144141
/// value. Replace all loads from the global address by invocations of a
145142
/// getter that returns the value of this variable.
@@ -735,8 +732,6 @@ void SILGlobalOpt::optimizeInitializer(SILFunction *AddrF,
735732
InitializerCount[InitF] > 1)
736733
return;
737734

738-
peepholeInitializer(InitF);
739-
740735
// If the globalinit_func is trivial, continue; otherwise bail.
741736
SingleValueInstruction *InitVal;
742737
SILGlobalVariable *SILG = getVariableOfStaticInitializer(InitF, InitVal);
@@ -759,58 +754,6 @@ void SILGlobalOpt::optimizeInitializer(SILFunction *AddrF,
759754
HasChanged = true;
760755
}
761756

762-
void SILGlobalOpt::peepholeInitializer(SILFunction *InitFunc) {
763-
if (InitFunc->size() != 1)
764-
return;
765-
SILBasicBlock *BB = &InitFunc->front();
766-
767-
for (auto &I : *BB) {
768-
if (auto *SI = dyn_cast<StoreInst>(&I)) {
769-
770-
// If struct S has a single field, replace
771-
// %a = struct_element_addr %s : $*S
772-
// store %x to %a
773-
// with
774-
// %y = struct $S (%x)
775-
// store %y to %s
776-
//
777-
// This pattern occurs with resilient static properties, like
778-
// struct ResilientStruct {
779-
// var singleField: Int
780-
// public static let x = ResilientStruct(singleField: 27)
781-
// }
782-
//
783-
// TODO: handle structs with multiple fields.
784-
SILValue Addr = SI->getDest();
785-
auto *SEA = dyn_cast<StructElementAddrInst>(Addr);
786-
if (!SEA)
787-
continue;
788-
789-
if (SEA->getOperand()->getType().isAddressOnly(InitFunc))
790-
continue;
791-
792-
StructDecl *Decl = SEA->getStructDecl();
793-
auto beginProp = Decl->getStoredProperties().begin();
794-
if (std::next(beginProp) != Decl->getStoredProperties().end())
795-
continue;
796-
797-
assert(*beginProp == SEA->getField());
798-
799-
SILBuilder B(SI);
800-
SILValue StructAddr = SEA->getOperand();
801-
StructInst *Struct = B.createStruct(SEA->getLoc(),
802-
StructAddr->getType().getObjectType(),
803-
{ SI->getSrc() });
804-
SI->setOperand(StoreInst::Src, Struct);
805-
SI->setOperand(StoreInst::Dest, StructAddr);
806-
if (SEA->use_empty()) {
807-
SEA->eraseFromParent();
808-
}
809-
HasChanged = true;
810-
}
811-
}
812-
}
813-
814757
static bool canBeChangedExternally(SILGlobalVariable *SILG) {
815758
// Don't assume anything about globals which are imported from other modules.
816759
if (isAvailableExternally(SILG->getLinkage()))

‎lib/SILOptimizer/SILCombiner/SILCombiner.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ class SILCombiner :
220220
SILInstruction *visitUpcastInst(UpcastInst *UCI);
221221
SILInstruction *optimizeLoadFromStringLiteral(LoadInst *LI);
222222
SILInstruction *visitLoadInst(LoadInst *LI);
223+
SILInstruction *visitStoreInst(StoreInst *si);
223224
SILInstruction *visitIndexAddrInst(IndexAddrInst *IA);
224225
SILInstruction *visitAllocStackInst(AllocStackInst *AS);
225226
SILInstruction *visitAllocRefInst(AllocRefInst *AR);

‎lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "swift/Basic/STLExtras.h"
1616
#include "swift/SIL/DebugUtils.h"
1717
#include "swift/SIL/DynamicCasts.h"
18+
#include "swift/SIL/InstructionUtils.h"
1819
#include "swift/SIL/PatternMatch.h"
1920
#include "swift/SIL/Projection.h"
2021
#include "swift/SIL/SILBuilder.h"
@@ -585,6 +586,79 @@ SILInstruction *SILCombiner::optimizeLoadFromStringLiteral(LoadInst *LI) {
585586
return Builder.createIntegerLiteral(LI->getLoc(), LI->getType(), str[index]);
586587
}
587588

589+
SILInstruction *SILCombiner::visitStoreInst(StoreInst *si) {
590+
auto *f = si->getFunction();
591+
assert(f->getConventions().useLoweredAddresses() &&
592+
"These optimizations assume that opaque values are not enabled");
593+
594+
// (store (struct_element_addr addr) object)
595+
// ->
596+
// (store addr (struct object))
597+
598+
// If our store's destination is not a struct_element_addr, bail early.
599+
auto *sea = dyn_cast<StructElementAddrInst>(si->getDest());
600+
if (!sea)
601+
return nullptr;
602+
603+
// Ok, we have at least one struct_element_addr. Canonicalize the underlying
604+
// store.
605+
Builder.setInsertionPoint(si);
606+
SILLocation loc = si->getLoc();
607+
608+
auto &mod = si->getModule();
609+
SILValue result = si->getSrc();
610+
SILValue iterAddr = sea->getOperand();
611+
SILValue storeAddr;
612+
while (true) {
613+
SILType iterAddrType = iterAddr->getType();
614+
615+
// If our aggregate has unreferenced storage then we can never prove if it
616+
// actually has a single field.
617+
if (iterAddrType.aggregateHasUnreferenceableStorage())
618+
break;
619+
620+
auto *decl = iterAddrType.getStructOrBoundGenericStruct();
621+
assert(
622+
!decl->isResilient(mod.getSwiftModule(), f->getResilienceExpansion()) &&
623+
"This code assumes resilient structs can not have fragile fields. If "
624+
"this assert is hit, this has been changed. Please update this code.");
625+
626+
// NOTE: If this is ever changed to support enums, we must check for address
627+
// only types here. For structs we do not have to check since a single
628+
// element struct with a loadable element can never be address only. We
629+
// additionally do not have to worry about our input value being address
630+
// only since we are storing into it.
631+
auto props = decl->getStoredProperties();
632+
if (std::next(props.begin()) != props.end())
633+
break;
634+
635+
// Update the store location now that we know it is safe.
636+
storeAddr = iterAddr;
637+
638+
// Otherwise, create the struct.
639+
result = Builder.createStruct(loc, iterAddrType.getObjectType(), result);
640+
641+
// See if we have another struct_element_addr we can strip off. If we don't
642+
// then this as much as we can promote.
643+
sea = dyn_cast<StructElementAddrInst>(sea->getOperand());
644+
if (!sea)
645+
break;
646+
iterAddr = sea->getOperand();
647+
}
648+
649+
// If we failed to create any structs, bail.
650+
if (result == si->getSrc())
651+
return nullptr;
652+
653+
// Then create a new store, storing the value into the relevant computed
654+
// address.
655+
Builder.createStore(loc, result, storeAddr,
656+
StoreOwnershipQualifier::Unqualified);
657+
658+
// Then eliminate the original store.
659+
return eraseInstFromFunction(*si);
660+
}
661+
588662
SILInstruction *SILCombiner::visitLoadInst(LoadInst *LI) {
589663
// (load (upcast-ptr %x)) -> (upcast-ref (load %x))
590664
Builder.setCurrentDebugScope(LI->getDebugScope());

‎test/SILOptimizer/sil_combine.sil

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,8 @@ bb3(%3 : $BoolLike):
285285
// CHECK-NEXT: [[IN_GEP:%[0-9]+]] = struct_element_addr [[IN]] : $*UInt8, #UInt8._value
286286
// CHECK-NEXT: [[IN_LOADED:%[0-9]+]] = load [[IN_GEP]] : $*Builtin.Int8
287287
// CHECK-NEXT: [[LITERAL:%[0-9]+]] = integer_literal $Builtin.Int8, 1
288-
// CHECK-NEXT: [[IN_GEP_STORE:%[0-9]+]] = struct_element_addr [[IN]] : $*UInt8, #UInt8._value
289-
// CHECK-NEXT: store [[LITERAL]] to [[IN_GEP_STORE]] : $*Builtin.Int8
288+
// CHECK-NEXT: [[UINT8:%.*]] = struct $UInt8 ([[LITERAL]] : $Builtin.Int8)
289+
// CHECK-NEXT: store [[UINT8]] to [[IN]] : $*UInt8
290290
// CHECK-NEXT: return [[IN_LOADED]] : $Builtin.Int8
291291
sil @struct_extract_load_to_load_struct_element_addr : $@convention(thin) (@inout UInt8) -> (Builtin.Int8) {
292292
bb0(%0 : $*UInt8):
@@ -322,7 +322,7 @@ bb0(%0 : $*(Builtin.Int8, Builtin.Int8)):
322322
// CHECK: bb0([[IN:%[0-9]+]] : $*UInt8):
323323
// CHECK-NEXT: load
324324
// CHECK-NEXT: integer_literal
325-
// CHECK-NEXT: struct_element_addr
325+
// CHECK-NEXT: struct
326326
// CHECK-NEXT: store
327327
// CHECK-NEXT: struct_extract
328328
// CHECK-NEXT: tuple

‎test/SILOptimizer/sil_combine_memopts.sil

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all -sil-combine %s | %FileCheck %s
2+
3+
// This file tests sil combine's canonicalization of memory.
4+
5+
class Klass {}
6+
7+
////////////////////////////
8+
// Store Canonicalization //
9+
////////////////////////////
10+
11+
// We canonicalize stores of fields of single element structs into stores of the
12+
// struct itself. The two are equivalent.
13+
14+
struct SingleEltStruct {
15+
var k: Klass
16+
}
17+
18+
// CHECK-LABEL: sil @promote_initialization_of_single_elt_struct : $@convention(thin) (@owned Klass) -> () {
19+
// CHECK: bb0([[ARG:%.*]] : $Klass):
20+
// CHECK: [[STACK:%.*]] = alloc_stack $SingleEltStruct
21+
// CHECK: [[STRUCT_ARG:%.*]] = struct $SingleEltStruct ([[ARG]] : $Klass)
22+
// CHECK: store [[STRUCT_ARG]] to [[STACK]]
23+
// CHECK: dealloc_stack [[STACK]]
24+
// CHECK: } // end sil function 'promote_initialization_of_single_elt_struct'
25+
sil @promote_initialization_of_single_elt_struct : $@convention(thin) (@owned Klass) -> () {
26+
bb0(%0 : $Klass):
27+
%1 = alloc_stack $SingleEltStruct
28+
%2 = struct_element_addr %1 : $*SingleEltStruct, #SingleEltStruct.k
29+
store %0 to %2 : $*Klass
30+
dealloc_stack %1 : $*SingleEltStruct
31+
%9999 = tuple()
32+
return %9999 : $()
33+
}
34+
35+
struct RecursiveSingleEltStruct {
36+
var field: RecursiveSingleEltStructField
37+
}
38+
39+
struct RecursiveSingleEltStructField {
40+
var k: Klass
41+
}
42+
43+
// CHECK-LABEL: sil @promote_initialization_of_recursive_single_elt_struct : $@convention(thin) (@owned Klass) -> () {
44+
// CHECK: bb0([[ARG:%.*]] : $Klass):
45+
// CHECK: [[STACK:%.*]] = alloc_stack $RecursiveSingleEltStruct
46+
// CHECK: [[STRUCT_ARG_1:%.*]] = struct $RecursiveSingleEltStructField ([[ARG]] : $Klass)
47+
// CHECK: [[STRUCT_ARG_2:%.*]] = struct $RecursiveSingleEltStruct ([[STRUCT_ARG_1]] : $RecursiveSingleEltStructField)
48+
// CHECK: store [[STRUCT_ARG_2]] to [[STACK]]
49+
// CHECK: dealloc_stack [[STACK]]
50+
// CHECK: } // end sil function 'promote_initialization_of_recursive_single_elt_struct'
51+
sil @promote_initialization_of_recursive_single_elt_struct : $@convention(thin) (@owned Klass) -> () {
52+
bb0(%0 : $Klass):
53+
%1 = alloc_stack $RecursiveSingleEltStruct
54+
%2 = struct_element_addr %1 : $*RecursiveSingleEltStruct, #RecursiveSingleEltStruct.field
55+
%3 = struct_element_addr %2 : $*RecursiveSingleEltStructField, #RecursiveSingleEltStructField.k
56+
store %0 to %3 : $*Klass
57+
%4 = load %1 : $*RecursiveSingleEltStruct
58+
release_value %4 : $RecursiveSingleEltStruct
59+
dealloc_stack %1 : $*RecursiveSingleEltStruct
60+
%9999 = tuple()
61+
return %9999 : $()
62+
}
63+
64+
struct MultipleFieldStruct {
65+
var k: Klass
66+
var field: RecursiveSingleEltStructField
67+
}
68+
69+
// CHECK-LABEL: sil @only_promote_as_far_as_have_single_elts : $@convention(thin) (@owned Klass) -> () {
70+
// CHECK: bb0([[ARG:%.*]] :
71+
// CHECK: [[STACK:%.*]] = alloc_stack $MultipleFieldStruct
72+
// CHECK: [[MULTIPLE_FIELD_SEA:%.*]] = struct_element_addr [[STACK]]
73+
// CHECK: [[VALUE:%.*]] = struct $RecursiveSingleEltStructField ([[ARG]] : $Klass)
74+
// CHECK: store [[VALUE]] to [[MULTIPLE_FIELD_SEA]]
75+
// CHECK: dealloc_stack [[STACK]]
76+
// CHECK: } // end sil function 'only_promote_as_far_as_have_single_elts'
77+
sil @only_promote_as_far_as_have_single_elts : $@convention(thin) (@owned Klass) -> () {
78+
bb0(%0 : $Klass):
79+
%1 = alloc_stack $MultipleFieldStruct
80+
%2 = struct_element_addr %1 : $*MultipleFieldStruct, #MultipleFieldStruct.field
81+
%3 = struct_element_addr %2 : $*RecursiveSingleEltStructField, #RecursiveSingleEltStructField.k
82+
store %0 to %3 : $*Klass
83+
dealloc_stack %1 : $*MultipleFieldStruct
84+
%9999 = tuple()
85+
return %9999 : $()
86+
}
87+
88+
// CHECK-LABEL: sil @perform_no_work_if_multiple_fields : $@convention(thin) (@owned RecursiveSingleEltStructField) -> () {
89+
// CHECK: bb0([[ARG:%.*]] :
90+
// CHECK: [[STACK:%.*]] = alloc_stack $MultipleFieldStruct
91+
// CHECK: [[MULTIPLE_FIELD_SEA:%.*]] = struct_element_addr [[STACK]]
92+
// CHECK: store [[ARG]] to [[MULTIPLE_FIELD_SEA]]
93+
// CHECK: dealloc_stack [[STACK]]
94+
// CHECK: } // end sil function 'perform_no_work_if_multiple_fields'
95+
sil @perform_no_work_if_multiple_fields : $@convention(thin) (@owned RecursiveSingleEltStructField) -> () {
96+
bb0(%0 : $RecursiveSingleEltStructField):
97+
%1 = alloc_stack $MultipleFieldStruct
98+
%2 = struct_element_addr %1 : $*MultipleFieldStruct, #MultipleFieldStruct.field
99+
store %0 to %2 : $*RecursiveSingleEltStructField
100+
dealloc_stack %1 : $*MultipleFieldStruct
101+
%9999 = tuple()
102+
return %9999 : $()
103+
}
104+
105+
// CHECK-LABEL: sil @only_promote_while_we_have_sea : $@convention(thin) (@owned Klass) -> () {
106+
// CHECK: bb0([[ARG:%.*]] :
107+
// CHECK: [[STACK:%.*]] = alloc_stack $(Klass, RecursiveSingleEltStructField)
108+
// CHECK: [[TUPLE_ADDR:%.*]] = tuple_element_addr [[STACK]]
109+
// CHECK: [[VALUE:%.*]] = struct $RecursiveSingleEltStructField ([[ARG]] : $Klass)
110+
// CHECK: store [[VALUE]] to [[TUPLE_ADDR]]
111+
// CHECK: dealloc_stack [[STACK]]
112+
// CHECK: } // end sil function 'only_promote_while_we_have_sea'
113+
sil @only_promote_while_we_have_sea : $@convention(thin) (@owned Klass) -> () {
114+
bb0(%0 : $Klass):
115+
%1 = alloc_stack $(Klass, RecursiveSingleEltStructField)
116+
%2 = tuple_element_addr %1 : $*(Klass, RecursiveSingleEltStructField), 1
117+
%3 = struct_element_addr %2 : $*RecursiveSingleEltStructField, #RecursiveSingleEltStructField.k
118+
store %0 to %3 : $*Klass
119+
dealloc_stack %1 : $*(Klass, RecursiveSingleEltStructField)
120+
%9999 = tuple()
121+
return %9999 : $()
122+
}

0 commit comments

Comments
 (0)