Merge pull request #11561 from swiftix/optimizer-improvements

A set of small optimizer improvements in preparation for the early SIL module serialization

Author: swiftix

Diff for: lib/SILOptimizer/LoopTransforms/LoopUnroll.cpp

@@ -19,6 +19,7 @@
 #include "swift/SILOptimizer/Analysis/LoopAnalysis.h"
 #include "swift/SILOptimizer/PassManager/Passes.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
+#include "swift/SILOptimizer/Utils/PerformanceInlinerUtils.h"
 #include "swift/SILOptimizer/Utils/SILInliner.h"
 #include "swift/SILOptimizer/Utils/SILSSAUpdater.h"
 
@@ -185,12 +186,24 @@ static bool canAndShouldUnrollLoop(SILLoop *Loop, uint64_t TripCount) {
 
   // We can unroll a loop if we can duplicate the instructions it holds.
   uint64_t Cost = 0;
+  // Average number of instructions per basic block.
+  // It is used to estimate the cost of the callee
+  // inside a loop.
+  const uint64_t InsnsPerBB = 4;
   for (auto *BB : Loop->getBlocks()) {
     for (auto &Inst : *BB) {
       if (!Loop->canDuplicate(&Inst))
         return false;
       if (instructionInlineCost(Inst) != InlineCost::Free)
         ++Cost;
+      if (auto AI = FullApplySite::isa(&Inst)) {
+        auto Callee = AI.getCalleeFunction();
+        if (Callee && getEligibleFunction(AI, InlineSelection::Everything)) {
+          // If callee is rather big and potentialy inlineable, it may be better
+          // not to unroll, so that the body of the calle can be inlined later.
+          Cost += Callee->size() * InsnsPerBB;
+        }
+      }
       if (Cost * TripCount > SILLoopUnrollThreshold)
         return false;
   }

Diff for: lib/SILOptimizer/Mandatory/ConstantPropagation.cpp

@@ -80,6 +80,11 @@ constantFoldBinaryWithOverflow(BuiltinInst *BI, llvm::Intrinsic::ID ID,
   // If we can statically determine that the operation overflows,
   // warn about it if warnings are not disabled by ResultsInError being null.
   if (ResultsInError.hasValue() && Overflow && ReportOverflow) {
+    if (BI->getFunction()->isSpecialization()) {
+      // Do not report any constant propagation issues in specializations,
+      // because they are eventually not present in the original function.
+      return nullptr;
+    }
     // Try to infer the type of the constant expression that the user operates
     // on. If the intrinsic was lowered from a call to a function that takes
     // two arguments of the same type, use the type of the LHS argument.

Diff for: lib/SILOptimizer/PassManager/PassPipeline.cpp

@@ -233,6 +233,9 @@ void addSSAPasses(SILPassPipelinePlan &P, OptimizationLevelKind OpLevel) {
   // current function (after optimizing any new callees).
   P.addDevirtualizer();
   P.addGenericSpecializer();
+  // Run devirtualizer after the specializer, because many
+  // class_method/witness_method instructions may use concrete types now.
+  P.addDevirtualizer();
 
   switch (OpLevel) {
   case OptimizationLevelKind::HighLevel:
@@ -287,6 +290,11 @@ void addSSAPasses(SILPassPipelinePlan &P, OptimizationLevelKind OpLevel) {
   } else
     P.addEarlyCodeMotion();
 
+  P.addRetainSinking();
+  // Retain sinking does not sink all retains in one round.
+  // Let it run one more time time, because it can be beneficial.
+  // FIXME: Improve the RetainSinking pass to sink more/all
+  // retains in one go.
   P.addRetainSinking();
   P.addReleaseHoisting();
   P.addARCSequenceOpts();

Diff for: lib/SILOptimizer/Transforms/PerformanceInliner.cpp

@@ -194,11 +194,16 @@ bool SILPerformanceInliner::isProfitableToInline(FullApplySite AI,
     BaseBenefit = BaseBenefit / 2;
   }
 
+  // It is always OK to inline a simple call.
+  // TODO: May be consider also the size of the callee?
+  if (isPureCall(AI, SEA))
+    return true;
+
   // Bail out if this generic call can be optimized by means of
   // the generic specialization, because we prefer generic specialization
   // to inlining of generics.
   if (IsGeneric && canSpecializeGeneric(AI, Callee, AI.getSubstitutions())) {
-    return isPureCall(AI, SEA);
+    return false;
   }
 
   SILLoopInfo *LI = LA->get(Callee);
@@ -338,7 +343,7 @@ bool SILPerformanceInliner::isProfitableToInline(FullApplySite AI,
     // Only inline trivial functions into thunks (which will not increase the
     // code size).
     if (CalleeCost > TrivialFunctionThreshold) {
-      return isPureCall(AI, SEA);
+      return false;
     }
 
     DEBUG(
@@ -359,7 +364,7 @@ bool SILPerformanceInliner::isProfitableToInline(FullApplySite AI,
 
   // This is the final inlining decision.
   if (CalleeCost > Benefit) {
-    return isPureCall(AI, SEA);
+    return false;
   }
 
   NumCallerBlocks += Callee->size();

Diff for: lib/SILOptimizer/Utils/PerformanceInlinerUtils.cpp

@@ -756,6 +756,10 @@ static bool isConstantValue(SILValue V) {
     }
     return true;
   }
+  if (auto *MT = dyn_cast<MetatypeInst>(V)) {
+    if (!MT->getType().hasArchetype())
+      return true;
+  }
   return false;
 }
 

Diff for: test/SILOptimizer/inline_heuristics.sil

@@ -223,26 +223,25 @@ bb0(%0 : $Int32):
 // CHECK-LOG-LABEL: Inline into caller: testCondBr
 // CHECK-LOG-NEXT: decision {{.*}}, b=50,
 
-sil @testCondBr : $@convention(thin) () -> () {
-bb0:
-
-  %0 = function_ref @condBrCallee : $@convention(thin) (Int32) -> Int32
+sil @testCondBr : $@convention(thin) (Int32) -> () {
+bb0(%a : $Int32):
+  %0 = function_ref @condBrCallee : $@convention(thin) (Int32, Int32) -> Int32
   %1 = integer_literal $Builtin.Int32, 27
   %2 = struct $Int32 (%1 : $Builtin.Int32)
-  %3 = apply %0(%2) : $@convention(thin) (Int32) -> Int32
+  %3 = apply %0(%2, %a) : $@convention(thin) (Int32, Int32) -> Int32
   %4 = tuple ()
   return %4 : $()
 }
 
-sil @condBrCallee : $@convention(thin) (Int32) -> Int32 {
-bb0(%0 : $Int32):
+sil @condBrCallee : $@convention(thin) (Int32, Int32) -> Int32 {
+bb0(%0 : $Int32, %r : $Int32):
   %1 = integer_literal $Builtin.Int32, 27
   %2 = struct_extract %0 : $Int32, #Int32._value
   %3 = builtin "cmp_eq_Word"(%2 : $Builtin.Int32, %1 : $Builtin.Int32) : $Builtin.Int1
   cond_br %3, bb1, bb2
 
 bb1:
-  br bb3(%0 : $Int32)
+  br bb3(%r : $Int32)
 
 bb2:
   // increase the scope length
@@ -263,24 +262,24 @@ bb3(%8 : $Int32):
 // CHECK: return %{{.*}} : $()
 
 // CHECK-LOG-LABEL: Inline into caller: testSwitchValue
-// CHECK-LOG-NEXT: decision {{.*}}, b=50,
+// CHECK-LOG-NEXT: decision {{.*}}, b=40,
 
-sil @testSwitchValue : $@convention(thin) () -> () {
-bb0:
+sil @testSwitchValue : $@convention(thin) (Int32) -> () {
+bb0(%a : $Int32):
 
-  %0 = function_ref @switchValueCallee : $@convention(thin) (Int32) -> Int32
+  %0 = function_ref @switchValueCallee : $@convention(thin) (Int32, Int32) -> Int32
   %1 = integer_literal $Builtin.Int32, 28
   %2 = struct $Int32 (%1 : $Builtin.Int32)
-  %3 = apply %0(%2) : $@convention(thin) (Int32) -> Int32
+  %3 = apply %0(%2, %a) : $@convention(thin) (Int32, Int32) -> Int32
   %4 = tuple ()
   return %4 : $()
 }
 
-sil @switchValueCallee : $@convention(thin) (Int32) -> Int32 {
-bb0(%0 : $Int32):
+sil @switchValueCallee : $@convention(thin) (Int32, Int32) -> Int32 {
+bb0(%0 : $Int32, %r : $Int32):
   %1 = integer_literal $Builtin.Int32, 27
   %2 = integer_literal $Builtin.Int32, 28
-  %3 = struct_extract %0 : $Int32, #Int32._value
+  %3 = struct_extract %r : $Int32, #Int32._value
   switch_value %3 : $Builtin.Int32, case %1 : bb1, case %2 : bb2, default bb3
 
 bb1:

Diff for: test/SILOptimizer/loop_unroll.sil

@@ -210,3 +210,74 @@ bb3:
   %8 = tuple()
   return %8 : $()
 }
+
+sil @big_func: $@convention(thin) () -> Builtin.Int64 {
+bb0:
+   %x0 = integer_literal $Builtin.Int64, 1
+   %overflow_check = integer_literal $Builtin.Int1, 0
+   %sum1 = builtin "sadd_with_overflow_Int64"(%x0 : $Builtin.Int64, %x0 : $Builtin.Int64, %overflow_check : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+   %x1 = tuple_extract %sum1 : $(Builtin.Int64, Builtin.Int1), 0
+   br bb1
+
+bb1:
+   %sum2 = builtin "sadd_with_overflow_Int64"(%x1 : $Builtin.Int64, %x1 : $Builtin.Int64, %overflow_check : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+   %x2 = tuple_extract %sum2 : $(Builtin.Int64, Builtin.Int1), 0
+   br bb2
+
+bb2:
+   %sum3 = builtin "sadd_with_overflow_Int64"(%x2 : $Builtin.Int64, %x2 : $Builtin.Int64, %overflow_check : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+   %x3 = tuple_extract %sum3 : $(Builtin.Int64, Builtin.Int1), 0
+   br bb3
+
+bb3:
+   %sum4 = builtin "sadd_with_overflow_Int64"(%x3 : $Builtin.Int64, %x3 : $Builtin.Int64, %overflow_check : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+   %x4 = tuple_extract %sum4 : $(Builtin.Int64, Builtin.Int1), 0
+   br bb4
+
+bb4:
+   %sum5 = builtin "sadd_with_overflow_Int64"(%x4 : $Builtin.Int64, %x4 : $Builtin.Int64, %overflow_check : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+   %x5 = tuple_extract %sum5 : $(Builtin.Int64, Builtin.Int1), 0
+   br bb5
+
+bb5:
+   %sum6 = builtin "sadd_with_overflow_Int64"(%x5 : $Builtin.Int64, %x5 : $Builtin.Int64, %overflow_check : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+   %x6 = tuple_extract %sum6 : $(Builtin.Int64, Builtin.Int1), 0
+   br bb6
+
+bb6:
+   return %x6 : $Builtin.Int64 
+}
+
+// Check that the compiler does not unroll loops containing calls
+// of big inlineable functions.
+//
+// CHECK-LABEL: sil @unroll_with_apply
+// CHECK: apply
+// CHECK: // end sil function 'unroll_with_apply'
+sil @unroll_with_apply : $@convention(thin) () -> () {
+bb0:
+ %0 = integer_literal $Builtin.Int64, 0
+ %1 = integer_literal $Builtin.Int64, 1
+ %2 = integer_literal $Builtin.Int64, 20
+ %3 = integer_literal $Builtin.Int1, 1
+ %f = function_ref @big_func: $@convention(thin) () -> Builtin.Int64 
+ br bb1(%0 : $Builtin.Int64)
+
+bb1(%4 : $Builtin.Int64):
+  %r = apply %f() : $@convention(thin) () -> Builtin.Int64
+  br bb2
+
+bb2:
+  %5 = builtin "sadd_with_overflow_Int64"(%4 : $Builtin.Int64, %1 : $Builtin.Int64, %3 : $Builtin.Int1) : $(Builtin.Int64, Builtin.Int1)
+  %6 = tuple_extract %5 : $(Builtin.Int64, Builtin.Int1), 0
+  %7 = builtin "cmp_eq_Int64"(%6 : $Builtin.Int64, %2 : $Builtin.Int64) : $Builtin.Int1
+  cond_br %7, bb4, bb3
+
+bb3:
+  br bb1(%6 : $Builtin.Int64)
+
+bb4:
+  %8 = tuple()
+  return %8 : $()
+}
+

Diff for: test/SILOptimizer/retain_release_code_motion.sil

@@ -1,4 +1,5 @@
 // RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all -retain-sinking -late-release-hoisting %s | %FileCheck %s
+// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all -retain-sinking -retain-sinking -late-release-hoisting %s | %FileCheck --check-prefix=CHECK-MULTIPLE-RS-ROUNDS %s
 
 import Builtin
 import Swift
@@ -59,6 +60,8 @@ enum Optional<T> {
   case some(T)
 }
 
+sil @createS : $@convention(thin) () -> @owned S
+
 sil @use_C2 : $@convention(thin) (C2) -> ()
 sil @user : $@convention(thin) (Builtin.NativeObject) -> ()
 sil @user_int : $@convention(thin) (Int) -> ()
@@ -531,3 +534,72 @@ bb3:
   %1 = tuple()
   return %1 : $()
 }
+
+/// Check that retain sinking needs multiple-passes to sink 2 retains.
+
+/// One round of retain-sinking can sink only one of retains.
+/// CHECK-LABEL: sil @checkRetainSinkingMultipleRounds
+/// CHECK:      bb9:
+/// CHECK-NEXT:   retain_value %2 : $S
+/// CHECK-NEXT:   release_value %2 : $S
+/// CHECK-NEXT:   release_value %2 : $S
+/// In the ideal world, we should see a third retain_value here.
+/// But it would require another round of retain sinking.
+/// CHECK-NEXT:   br bb5
+
+
+/// Two rounds of retain-sinking can sink only two retains.
+/// CHECK-MULTIPLE-RS-ROUNDS-LABEL: sil @checkRetainSinkingMultipleRounds
+/// CHECK-MULTIPLE-RS-ROUNDS:      bb9:
+/// CHECK-MULTIPLE-RS-ROUNDS-NEXT:   retain_value %2 : $S
+/// CHECK-MULTIPLE-RS-ROUNDS-NEXT:   retain_value %2 : $S
+/// CHECK-MULTIPLE-RS-ROUNDS-NEXT:   release_value %2 : $S
+/// CHECK-MULTIPLE-RS-ROUNDS-NEXT:   release_value %2 : $S
+/// CHECK-MULTIPLE-RS-ROUNDS-NEXT:   br bb5
+
+sil @checkRetainSinkingMultipleRounds : $@convention(thin) (Int) -> () {
+bb0(%0 : $Int):
+  %1 = function_ref @createS : $@convention(thin) () -> @owned S
+  %2 = apply %1() : $@convention(thin) () -> @owned S
+  br bb2
+
+bb1: 
+  cond_br undef, bb10, bb11 
+
+bb2: 
+  cond_br undef, bb4, bb6
+
+bb3:
+  br bb2
+
+bb4:
+  retain_value %2 : $S
+  br bb5
+
+bb5:
+  release_value %2 : $S
+  cond_br undef, bb1, bb3
+
+bb6:
+  retain_value %2 : $S
+  retain_value %2 : $S
+  br bb7
+
+bb7:
+  cond_br undef, bb9, bb8
+
+bb8:
+  br bb7
+
+bb9:
+  release_value %2 : $S
+  br bb5
+
+bb10:
+  unreachable
+
+bb11:
+  release_value %2 : $S
+  %26 = tuple ()
+  return %26 : $()
+}