Merge pull request #20207 from atrick/fix-letprop
Fix static initializer analysis, used in LetPropertiesOpt.
diff --git a/lib/SILOptimizer/IPO/GlobalOpt.cpp b/lib/SILOptimizer/IPO/GlobalOpt.cpp
index a0b2d19..9ee51ec 100644
--- a/lib/SILOptimizer/IPO/GlobalOpt.cpp
+++ b/lib/SILOptimizer/IPO/GlobalOpt.cpp
@@ -292,13 +292,10 @@
auto V = Store->getSrc();
SmallVector<SILInstruction *, 8> Insts;
- Insts.push_back(Store);
- Insts.push_back(cast<SingleValueInstruction>(Store->getDest()));
if (!analyzeStaticInitializer(V, Insts))
return nullptr;
-
- // Produce a correct order of instructions.
- std::reverse(Insts.begin(), Insts.end());
+ Insts.push_back(cast<SingleValueInstruction>(Store->getDest()));
+ Insts.push_back(Store);
auto *GetterF = getGlobalGetterFunction(FunctionBuilder,
Store->getModule(),
diff --git a/lib/SILOptimizer/IPO/LetPropertiesOpts.cpp b/lib/SILOptimizer/IPO/LetPropertiesOpts.cpp
index 5de9e22..39512cd 100644
--- a/lib/SILOptimizer/IPO/LetPropertiesOpts.cpp
+++ b/lib/SILOptimizer/IPO/LetPropertiesOpts.cpp
@@ -401,7 +401,6 @@
// Analyze the init value being stored by the instruction into a property.
bool
LetPropertiesOpt::analyzeInitValue(SILInstruction *I, VarDecl *Property) {
- SmallVector<SILInstruction *, 8> ReverseInsns;
SILValue value;
if (auto SI = dyn_cast<StructInst>(I)) {
value = SI->getFieldValue(Property);
@@ -422,16 +421,10 @@
}
// Bail if a value of a property is not a statically known constant init.
- if (!analyzeStaticInitializer(value, ReverseInsns))
- return false;
-
- // Fill in the InitSequence by reversing the instructions and
- // setting the result index.
InitSequence sequence;
- while (!ReverseInsns.empty()) {
- sequence.Instructions.push_back(ReverseInsns.pop_back_val());
- }
sequence.Result = value;
+ if (!analyzeStaticInitializer(value, sequence.Instructions))
+ return false;
auto &cachedSequence = InitMap[Property];
if (cachedSequence.isValid() &&
diff --git a/lib/SILOptimizer/Utils/Local.cpp b/lib/SILOptimizer/Utils/Local.cpp
index de1ddd2..559912c 100644
--- a/lib/SILOptimizer/Utils/Local.cpp
+++ b/lib/SILOptimizer/Utils/Local.cpp
@@ -1411,53 +1411,94 @@
return true;
}
-/// Check if the value of V is computed by means of a simple initialization.
-/// Store the actual SILValue into Val and the reversed list of instructions
-/// initializing it in Insns.
-///
-/// The check is performed by recursively walking the computation of the SIL
-/// value being analyzed.
-bool
-swift::analyzeStaticInitializer(SILValue V,
- SmallVectorImpl<SILInstruction *> &Insts) {
- // Save every instruction we see.
- // TODO: MultiValueInstruction?
- if (auto *I = dyn_cast<SingleValueInstruction>(V))
- Insts.push_back(I);
+// Encapsulate the state used for recursive analysis of a static
+// initializer. Discover all the instruction in a use-def graph and return them
+// in topological order.
+//
+// TODO: We should have a DFS utility for this sort of thing so it isn't
+// recursive.
+class StaticInitializerAnalysis {
+ SmallVectorImpl<SILInstruction *> &postOrderInstructions;
+ llvm::SmallDenseSet<SILValue, 8> visited;
+ int recursionLevel = 0;
- if (auto *SI = dyn_cast<StructInst>(V)) {
- // If it is not a struct which is a simple type, bail.
- if (!isSimpleType(SI->getType(), SI->getModule()))
- return false;
- return llvm::all_of(SI->getAllOperands(), [&](Operand &Op) -> bool {
- return analyzeStaticInitializer(Op.get(), Insts);
- });
+public:
+ StaticInitializerAnalysis(
+ SmallVectorImpl<SILInstruction *> &postOrderInstructions)
+ : postOrderInstructions(postOrderInstructions) {}
+
+ // Perform a recursive DFS on on the use-def graph rooted at `V`. Insert
+ // values in the `visited` set in preorder. Insert values in
+ // `postOrderInstructions` in postorder so that the instructions are
+ // topologically def-use ordered (in execution order).
+ bool analyze(SILValue RootValue) {
+ return recursivelyAnalyzeOperand(RootValue);
}
- if (auto *TI = dyn_cast<TupleInst>(V)) {
- // If it is not a tuple which is a simple type, bail.
- if (!isSimpleType(TI->getType(), TI->getModule()))
+protected:
+ bool recursivelyAnalyzeOperand(SILValue V) {
+ if (!visited.insert(V).second)
+ return true;
+
+ if (++recursionLevel > 50)
return false;
- return llvm::all_of(TI->getAllOperands(), [&](Operand &Op) -> bool {
- return analyzeStaticInitializer(Op.get(), Insts);
- });
+
+ // TODO: For multi-result instructions, we could simply insert all result
+ // values in the visited set here.
+ auto *I = dyn_cast<SingleValueInstruction>(V);
+ if (!I)
+ return false;
+
+ if (!recursivelyAnalyzeInstruction(I))
+ return false;
+
+ postOrderInstructions.push_back(I);
+ --recursionLevel;
+ return true;
}
- if (auto *bi = dyn_cast<BuiltinInst>(V)) {
- switch (bi->getBuiltinInfo().ID) {
- case BuiltinValueKind::FPTrunc:
- if (auto *LI = dyn_cast<LiteralInst>(bi->getArguments()[0])) {
- return analyzeStaticInitializer(LI, Insts);
- }
- return false;
- default:
- return false;
+ bool recursivelyAnalyzeInstruction(SILInstruction *I) {
+ if (auto *SI = dyn_cast<StructInst>(I)) {
+ // If it is not a struct which is a simple type, bail.
+ if (!isSimpleType(SI->getType(), SI->getModule()))
+ return false;
+
+ return llvm::all_of(SI->getAllOperands(), [&](Operand &Op) -> bool {
+ return recursivelyAnalyzeOperand(Op.get());
+ });
}
- }
+ if (auto *TI = dyn_cast<TupleInst>(I)) {
+ // If it is not a tuple which is a simple type, bail.
+ if (!isSimpleType(TI->getType(), TI->getModule()))
+ return false;
- return isa<IntegerLiteralInst>(V)
- || isa<FloatLiteralInst>(V)
- || isa<StringLiteralInst>(V);
+ return llvm::all_of(TI->getAllOperands(), [&](Operand &Op) -> bool {
+ return recursivelyAnalyzeOperand(Op.get());
+ });
+ }
+ if (auto *bi = dyn_cast<BuiltinInst>(I)) {
+ switch (bi->getBuiltinInfo().ID) {
+ case BuiltinValueKind::FPTrunc:
+ if (auto *LI = dyn_cast<LiteralInst>(bi->getArguments()[0])) {
+ return recursivelyAnalyzeOperand(LI);
+ }
+ return false;
+ default:
+ return false;
+ }
+ }
+ return isa<IntegerLiteralInst>(I) || isa<FloatLiteralInst>(I)
+ || isa<StringLiteralInst>(I);
+ }
+};
+
+/// Check if the value of V is computed by means of a simple initialization.
+/// Populate `forwardInstructions` with references to all the instructions
+/// that participate in the use-def graph required to compute `V`. The
+/// instructions will be in def-use topological order.
+bool swift::analyzeStaticInitializer(
+ SILValue V, SmallVectorImpl<SILInstruction *> &forwardInstructions) {
+ return StaticInitializerAnalysis(forwardInstructions).analyze(V);
}
/// Replace load sequence which may contain
diff --git a/test/SILOptimizer/let_properties_opts.sil b/test/SILOptimizer/let_properties_opts.sil
new file mode 100644
index 0000000..3ff5296
--- /dev/null
+++ b/test/SILOptimizer/let_properties_opts.sil
@@ -0,0 +1,77 @@
+// RUN: %target-sil-opt -let-properties-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s | %FileCheck %s
+
+sil_stage canonical
+
+import Builtin
+import Swift
+
+// Test initialization of a constant aggregate "let".
+// <rdar://problem/45691574> [SR-9146] IBAnimatable - SILCloner crash in LetPropertiesOpt.
+
+struct Point {
+ var x: Int64
+ var y: Int64
+}
+class HasCenter {
+
+ let centerPoint: Point = Point(x: 0, y: 0)
+
+ public func getCenter() -> Int64
+}
+
+sil hidden @$s19let_properties_opts5PointV1x1yACSi_SitcfC : $@convention(method) (Int64, Int64, @thin Point.Type) -> Point {
+bb0(%0 : $Int64, %1 : $Int64, %2 : $@thin Point.Type):
+ %3 = struct $Point (%0 : $Int64, %1 : $Int64)
+ return %3 : $Point
+}
+
+// variable initialization expression of HasCenter.centerPoint
+sil hidden [transparent] @$s19let_properties_opts9HasCenterC11centerPointAA0E0Vvpfi : $@convention(thin) () -> Point {
+bb0:
+ %0 = integer_literal $Builtin.Int64, 0
+ %1 = struct $Int64 (%0 : $Builtin.Int64)
+ %2 = struct $Point (%1 : $Int64, %1 : $Int64)
+ return %2 : $Point
+}
+
+// HasCenter.centerPoint.getter
+// CHECK-LABEL: sil hidden [transparent] @$s19let_properties_opts9HasCenterC11centerPointAA0E0Vvg : $@convention(method) (@guaranteed HasCenter) -> Point {
+// CHECK: bb0(%0 : $HasCenter):
+// CHECK: [[LIT:%.*]] = integer_literal $Builtin.Int64, 0
+// CHECK: [[INT:%.*]] = struct $Int64 ([[LIT]] : $Builtin.Int64)
+// CHECK: [[PNT:%.*]] = struct $Point ([[INT]] : $Int64, [[INT]] : $Int64)
+// CHECK: return [[PNT]] : $Point
+// CHECK-LABEL: } // end sil function '$s19let_properties_opts9HasCenterC11centerPointAA0E0Vvg'
+sil hidden [transparent] @$s19let_properties_opts9HasCenterC11centerPointAA0E0Vvg : $@convention(method) (@guaranteed HasCenter) -> Point {
+bb0(%0 : $HasCenter):
+ %1 = ref_element_addr %0 : $HasCenter, #HasCenter.centerPoint
+ %2 = load %1 : $*Point
+ return %2 : $Point
+}
+
+// HasCenter.getCenter()
+// CHECK-LABEL: sil hidden @$s19let_properties_opts9HasCenterC9getCenterSiyF : $@convention(method) (@guaranteed HasCenter) -> Int64 {
+// CHECK: [[LIT:%.*]] = integer_literal $Builtin.Int64, 0
+// CHECK: [[INT:%.*]] = struct $Int64 ([[LIT]] : $Builtin.Int64)
+// CHECK: [[PNT:%.*]] = struct $Point ([[INT]] : $Int64, [[INT]] : $Int64)
+// CHECK: [[X:%.*]] = struct_extract [[PNT]] : $Point, #Point.x
+// CHECK: return [[X]] : $Int64
+// CHECK-LABEL: } // end sil function '$s19let_properties_opts9HasCenterC9getCenterSiyF'
+sil hidden @$s19let_properties_opts9HasCenterC9getCenterSiyF : $@convention(method) (@guaranteed HasCenter) -> Int64 {
+bb0(%0 : $HasCenter):
+ %1 = ref_element_addr %0 : $HasCenter, #HasCenter.centerPoint
+ %2 = struct_element_addr %1 : $*Point, #Point.x
+ %3 = load %2 : $*Int64
+ return %3 : $Int64
+}
+
+// HasCenter.init()
+sil hidden @$s19let_properties_opts9HasCenterCACycfc : $@convention(method) (@owned HasCenter) -> @owned HasCenter {
+bb0(%0 : $HasCenter):
+ %1 = integer_literal $Builtin.Int64, 0
+ %2 = struct $Int64 (%1 : $Builtin.Int64)
+ %3 = struct $Point (%2 : $Int64, %2 : $Int64)
+ %4 = ref_element_addr %0 : $HasCenter, #HasCenter.centerPoint
+ store %3 to %4 : $*Point
+ return %0 : $HasCenter
+}
