Merge pull request #62000 from eeckstein/side-effects-in-inliner

Use the new side effect analysis in the performance inliner

Author: eeckstein

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeSideEffects.swift

@@ -44,23 +44,16 @@ let computeSideEffects = FunctionPass(name: "compute-side-effects", {
 
   var collectedEffects = CollectedEffects(function: function, context)
 
-  var deadEndBlocks = DeadEndBlocks(function: function, context)
-  defer { deadEndBlocks.deinitialize() }
-  
   // First step: collect effects from all instructions.
   //
   for block in function.blocks {
-    // Effects in blocks from which the function doesn't return are not relevant for the caller.
-    if deadEndBlocks.isDeadEnd(block: block) {
-      continue
-    }
     for inst in block.instructions {
       collectedEffects.addInstructionEffects(inst)
     }
   }
 
   // Second step: If an argument has unknown uses, we must add all previously collected
-  // global effects to the argument, because we don't know to wich "global" side-effect
+  // global effects to the argument, because we don't know to which "global" side-effect
   // instruction the argument might have escaped.
   for argument in function.arguments {
     collectedEffects.addEffectsForEcapingArgument(argument: argument)
@@ -380,7 +373,7 @@ private struct ArgumentEscapingWalker : ValueDefUseWalker, AddressDefUseWalker {
   mutating func hasUnknownUses(argument: FunctionArgument) -> Bool {
     if argument.type.isAddress {
       return walkDownUses(ofAddress: argument, path: UnusedWalkingPath()) == .abortWalk
-    } else if argument.hasTrivialType {
+    } else if argument.hasTrivialNonPointerType {
       return false
     } else {
       return walkDownUses(ofValue: argument, path: UnusedWalkingPath()) == .abortWalk

SwiftCompilerSources/Sources/Optimizer/TestPasses/EscapeInfoDumper.swift

@@ -130,11 +130,24 @@ let addressEscapeInfoDumper = FunctionPass(name: "dump-addr-escape-info", {
         let projLhs = lhs.at(AliasAnalysis.getPtrOrAddressPath(for: lhs))
         let projRhs = rhs.at(AliasAnalysis.getPtrOrAddressPath(for: rhs))
         let mayAlias = projLhs.canAddressAlias(with: projRhs, context)
-        assert(mayAlias == projRhs.canAddressAlias(with: projLhs, context),
-               "canAddressAlias(with:) must be symmetric")
+        if mayAlias != projRhs.canAddressAlias(with: projLhs, context) {
+          fatalError("canAddressAlias(with:) must be symmetric")
+        }
 
+        let addrReachable: Bool
+        if lhs.type.isAddress && !rhs.type.isAddress {
+          let anythingReachableFromRhs = rhs.at(SmallProjectionPath(.anything))
+          addrReachable = projLhs.canAddressAlias(with: anythingReachableFromRhs, context)
+          if mayAlias && !addrReachable {
+            fatalError("mayAlias implies addrReachable")
+          }
+        } else {
+          addrReachable = false
+        }
         if mayAlias {
           print("may alias")
+        } else if addrReachable {
+          print("address reachable but no alias")
         } else {
           print("no alias")
         }

SwiftCompilerSources/Sources/SIL/Effects.swift

@@ -128,6 +128,14 @@ extension Function {
     default:
       break
     }
+    if isProgramTerminationPoint {
+      // We can ignore any memory writes in a program termination point, because it's not relevant
+      // for the caller. But we need to consider memory reads, otherwise preceeding memory writes
+      // would be eliminated by dead-store-elimination in the caller. E.g. String initialization
+      // for error strings which are printed by the program termination point.
+      // Regarding ownership: a program termination point must not touch any reference counted objects.
+      return SideEffects.GlobalEffects(memory: SideEffects.Memory(read: true))
+    }
     var result = SideEffects.GlobalEffects.worstEffects
     switch effectAttribute {
     case .none:

SwiftCompilerSources/Sources/SIL/Function.swift

@@ -121,6 +121,10 @@ final public class Function : CustomStringConvertible, HasShortDescription, Hash
     }
   }
 
+  /// True if the callee function is annotated with @_semantics("programtermination_point").
+  /// This means that the function terminates the program.
+  public var isProgramTerminationPoint: Bool { hasSemanticsAttribute("programtermination_point") }
+
   /// Kinds of effect attributes which can be defined for a Swift function.
   public enum EffectAttribute {
     /// No effect attribute is specified.

include/swift/SILOptimizer/Utils/PerformanceInlinerUtils.h

@@ -28,7 +28,7 @@ using namespace swift;
 extern llvm::cl::opt<bool> EnableSILInliningOfGenerics;
 
 namespace swift {
-class SideEffectAnalysis;
+class BasicCalleeAnalysis;
 
 // Controls the decision to inline functions with @_semantics, @effect and
 // global_init attributes.
@@ -45,7 +45,7 @@ SILFunction *getEligibleFunction(FullApplySite AI,
 
 // Returns true if this is a pure call, i.e. the callee has no side-effects
 // and all arguments are constants.
-bool isPureCall(FullApplySite AI, SideEffectAnalysis *SEA);
+bool isPureCall(FullApplySite AI, BasicCalleeAnalysis *BCA);
 
 /// Fundamental @_semantic tags provide additional semantics for optimization
 /// beyond what SIL analysis can discover from the function body. They should be

lib/SILOptimizer/Transforms/PerformanceInliner.cpp

@@ -15,7 +15,7 @@
 #include "swift/AST/SemanticAttrs.h"
 #include "swift/SIL/MemAccessUtils.h"
 #include "swift/SIL/OptimizationRemark.h"
-#include "swift/SILOptimizer/Analysis/SideEffectAnalysis.h"
+#include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
 #include "swift/SILOptimizer/PassManager/Passes.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
 #include "swift/SILOptimizer/Utils/CFGOptUtils.h"
@@ -120,7 +120,7 @@ class SILPerformanceInliner {
 
   DominanceAnalysis *DA;
   SILLoopAnalysis *LA;
-  SideEffectAnalysis *SEA;
+  BasicCalleeAnalysis *BCA;
 
   // For keys of SILFunction and SILLoop.
   llvm::DenseMap<SILFunction *, ShortestPathAnalysis *> SPAs;
@@ -246,10 +246,10 @@ class SILPerformanceInliner {
 public:
   SILPerformanceInliner(StringRef PassName, SILOptFunctionBuilder &FuncBuilder,
                         InlineSelection WhatToInline, DominanceAnalysis *DA,
-                        SILLoopAnalysis *LA, SideEffectAnalysis *SEA,
+                        SILLoopAnalysis *LA, BasicCalleeAnalysis *BCA,
                         OptimizationMode OptMode, OptRemark::Emitter &ORE)
       : PassName(PassName), FuncBuilder(FuncBuilder),
-        WhatToInline(WhatToInline), DA(DA), LA(LA), SEA(SEA), CBI(DA), ORE(ORE),
+        WhatToInline(WhatToInline), DA(DA), LA(LA), BCA(BCA), CBI(DA), ORE(ORE),
         OptMode(OptMode) {}
 
   bool inlineCallsIntoFunction(SILFunction *F);
@@ -352,7 +352,7 @@ bool SILPerformanceInliner::isProfitableToInline(
 
   // It is always OK to inline a simple call.
   // TODO: May be consider also the size of the callee?
-  if (isPureCall(AI, SEA)) {
+  if (isPureCall(AI, BCA)) {
     OptRemark::Emitter::emitOrDebug(DEBUG_TYPE, &ORE, [&]() {
       using namespace OptRemark;
       return RemarkPassed("Inline", *AI.getInstruction())
@@ -394,6 +394,7 @@ bool SILPerformanceInliner::isProfitableToInline(
   // We will only inline if *ALL* dynamic accesses are
   // known and have no nested conflict
   bool AllAccessesBeneficialToInline = true;
+  bool returnsAllocation = false;
 
   // Calculate the inlining cost of the callee.
   int CalleeCost = 0;
@@ -510,6 +511,18 @@ bool SILPerformanceInliner::isProfitableToInline(
             AllAccessesBeneficialToInline = false;
           }
         }
+      } else if (auto ri = dyn_cast<ReturnInst>(&I)) {
+        SILValue retVal = ri->getOperand();
+        if (auto *uci = dyn_cast<UpcastInst>(retVal))
+          retVal = uci->getOperand();
+
+        // Inlining functions which return an allocated object or partial_apply
+        // most likely has a benefit in the caller, because e.g. it can enable
+        // de-virtualization.
+        if (isa<AllocationInst>(retVal) || isa<PartialApplyInst>(retVal)) {
+          BlockW.updateBenefit(Benefit, RemovedCallBenefit + 10);
+          returnsAllocation = true;
+        }
       }
     }
     // Don't count costs in blocks which are dead after inlining.
@@ -577,6 +590,8 @@ bool SILPerformanceInliner::isProfitableToInline(
 
   if (isClassMethodAtOsize && Benefit > OSizeClassMethodBenefit) {
     Benefit = OSizeClassMethodBenefit;
+    if (returnsAllocation)
+      Benefit += 10;
   }
 
   // This is the final inlining decision.
@@ -1110,7 +1125,7 @@ class SILPerformanceInlinerPass : public SILFunctionTransform {
   void run() override {
     DominanceAnalysis *DA = PM->getAnalysis<DominanceAnalysis>();
     SILLoopAnalysis *LA = PM->getAnalysis<SILLoopAnalysis>();
-    SideEffectAnalysis *SEA = PM->getAnalysis<SideEffectAnalysis>();
+    BasicCalleeAnalysis *BCA = PM->getAnalysis<BasicCalleeAnalysis>();
     OptRemark::Emitter ORE(DEBUG_TYPE, *getFunction());
 
     if (getOptions().InlineThreshold == 0) {
@@ -1122,7 +1137,7 @@ class SILPerformanceInlinerPass : public SILFunctionTransform {
     SILOptFunctionBuilder FuncBuilder(*this);
 
     SILPerformanceInliner Inliner(getID(), FuncBuilder, WhatToInline, DA, LA,
-                                  SEA, OptMode, ORE);
+                                  BCA, OptMode, ORE);
 
     assert(getFunction()->isDefinition() &&
            "Expected only functions with bodies!");

lib/SILOptimizer/Utils/PerformanceInlinerUtils.cpp

@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "swift/SILOptimizer/Analysis/ArraySemantic.h"
-#include "swift/SILOptimizer/Analysis/SideEffectAnalysis.h"
+#include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
 #include "swift/SILOptimizer/Utils/PerformanceInlinerUtils.h"
 #include "swift/AST/Module.h"
 #include "swift/SILOptimizer/Utils/InstOptUtils.h"
@@ -933,14 +933,11 @@ static bool isConstantArg(Operand *Arg) {
 }
 
 
-bool swift::isPureCall(FullApplySite AI, SideEffectAnalysis *SEA) {
+bool swift::isPureCall(FullApplySite AI, BasicCalleeAnalysis *BCA) {
   // If a call has only constant arguments and the call is pure, i.e. has
   // no side effects, then we should always inline it.
   // This includes arguments which are objects initialized with constant values.
-  FunctionSideEffects ApplyEffects;
-  SEA->getCalleeEffects(ApplyEffects, AI);
-  auto GE = ApplyEffects.getGlobalEffects();
-  if (GE.mayRead() || GE.mayWrite() || GE.mayRetain() || GE.mayRelease())
+  if (BCA->getMemoryBehavior(AI, /*observeRetains*/ true) != SILInstruction::MemoryBehavior::None)
     return false;
   // Check if all parameters are constant.
   auto Args = AI.getArgumentOperands().slice(AI.getNumIndirectSILResults());

test/SILOptimizer/addr_escape_info.sil

@@ -133,6 +133,44 @@ bb0:
   return %5 : $X
 }
 
+sil @return_x : $@convention(thin) (@guaranteed X) -> @owned X
+
+// CHECK-LABEL: Address escape information for test_class_argument:
+// CHECK:       pair 0 - 1
+// CHECK-NEXT:    %0 = alloc_ref $X
+// CHECK-NEXT:    %3 = ref_element_addr %2 : $X, #X.s
+// CHECK-NEXT:  may alias
+// CHECK:       End function test_class_argument
+sil @test_class_argument : $@convention(thin) () -> () {
+bb0:
+  %0 = alloc_ref $X
+  %1 = function_ref @return_x : $@convention(thin) (@guaranteed X) -> @owned X
+  %2 = apply %1(%0) : $@convention(thin) (@guaranteed X) -> @owned X
+  %3 = ref_element_addr %2 : $X, #X.s
+  fix_lifetime %0 : $X
+  fix_lifetime %3 : $*Str
+  %12 = tuple ()
+  return %12 : $()
+}
+
+// CHECK-LABEL: Address escape information for test_reachable_no_alias:
+// CHECK:       pair 0 - 1
+// CHECK-NEXT:    %3 = ref_element_addr %2 : $X, #X.s
+// CHECK-NEXT:    %0 = alloc_ref $XandIntClass
+// CHECK-NEXT:  address reachable but no alias
+// CHECK:       End function test_reachable_no_alias
+sil @test_reachable_no_alias : $@convention(thin) () -> () {
+bb0:
+  %0 = alloc_ref $XandIntClass
+  %1 = ref_element_addr %0 : $XandIntClass, #XandIntClass.x
+  %2 = load %1 : $*X
+  %3 = ref_element_addr %2 : $X, #X.s
+  fix_lifetime %3 : $*Str
+  fix_lifetime %0 : $XandIntClass
+  %12 = tuple ()
+  return %12 : $()
+}
+
 // CHECK-LABEL: Address escape information for test_struct_with_class:
 // CHECK:       value:  %1 = struct $Container (%0 : $X)
 // CHECK-NEXT:    ==>   %7 = apply %6(%1) : $@convention(thin) (@guaranteed Container) -> ()

test/SILOptimizer/assemblyvision_remark/attributes.swift

@@ -40,13 +40,13 @@ public func failMatch() {
 
 @_semantics("optremark")
 public func allocateInlineCallee() -> Klass {
-    return Klass() // expected-remark {{Pure call. Always profitable to inline "main.Klass.__allocating_init()"}}
+    return Klass() // expected-remark {{"main.Klass.__allocating_init()" inlined into "main.allocateInlineCallee()"}}
                    // expected-remark @-1 {{heap allocated ref of type 'Klass'}}
 }
 
 @_semantics("optremark.sil-inliner")
 public func allocateInlineCallee2() -> Klass {
-    return Klass() // expected-remark {{Pure call. Always profitable to inline "main.Klass.__allocating_init()"}}
+    return Klass() // expected-remark {{"main.Klass.__allocating_init()" inlined into "main.allocateInlineCallee2()"}}
 }
 
 // This makes sure we don't emit any remarks if we do not have semantics.
@@ -58,14 +58,14 @@ public func allocateInlineCallee3() -> Klass {
 @_semantics("optremark.sil-assembly-vision-remark-gen")
 public func mix1() -> (Klass, Klass) {
     let x = getGlobal()
-    return (x, Klass()) // expected-remark {{Pure call. Always profitable to inline "main.Klass.__allocating_init()"}}
+    return (x, Klass()) // expected-remark {{"main.Klass.__allocating_init()" inlined into "main.mix1()"}}
                         // expected-remark @-1:16 {{heap allocated ref of type 'Klass'}}
 }
 
 @_semantics("optremark.sil-inliner")
 public func mix2() -> (Klass, Klass) {
     let x = getGlobal()
-    return (x, Klass()) // expected-remark {{Pure call. Always profitable to inline "main.Klass.__allocating_init()"}}
+    return (x, Klass()) // expected-remark {{"main.Klass.__allocating_init()" inlined into "main.mix2()"}}
 }
 
 @_semantics("optremark.sil-assembly-vision-remark-gen")
@@ -77,7 +77,7 @@ public func mix3() -> (Klass, Klass) {
 @_semantics("optremark")
 public func mix4() -> (Klass, Klass) {
     let x = getGlobal()
-    return (x, Klass()) // expected-remark {{Pure call. Always profitable to inline "main.Klass.__allocating_init()"}}
+    return (x, Klass()) // expected-remark {{"main.Klass.__allocating_init()" inlined into "main.mix4()"}}
                         // expected-remark @-1 {{heap allocated ref of type 'Klass'}}
 }
 
@@ -89,6 +89,6 @@ public func mix5() -> (Klass, Klass) {
 @_assemblyVision
 public func mix4a() -> (Klass, Klass) {
     let x = getGlobal()
-    return (x, Klass()) // expected-remark {{Pure call. Always profitable to inline "main.Klass.__allocating_init()"}}
+    return (x, Klass()) // expected-remark {{"main.Klass.__allocating_init()" inlined into "main.mix4a()"}}
                         // expected-remark @-1 {{heap allocated ref of type 'Klass'}}
 }

test/SILOptimizer/inline_heuristics.sil

@@ -1,9 +1,11 @@
 // RUN: %empty-directory(%t)
-// RUN: %target-sil-opt -enable-sil-verify-all %s -inline -sil-inline-generics=true -sil-partial-specialization=false -debug-only=sil-inliner 2>%t/log | %FileCheck %s
+// RUN: %target-sil-opt -enable-sil-verify-all %s -compute-side-effects -inline -sil-inline-generics=true -sil-partial-specialization=false -debug-only=sil-inliner 2>%t/log | %FileCheck %s
 // RUN: %FileCheck %s --check-prefix=CHECK-LOG <%t/log
 // RUN: %target-sil-opt -enable-sil-verify-all %s -inline -sil-inline-generics=true -sil-partial-specialization=true -generic-specializer -debug-only=sil-inliner 2>%t/log | %FileCheck %s --check-prefix=CHECK-PARTIAL-SPECIALIZATION
 // REQUIRES: asserts
 
+// REQUIRES: swift_in_compiler
+
 // This test checks the inline heuristics based on the debug log output of
 // the performance inliner.
 // Checking the final sil is just a goodie.

test/SILOptimizer/inliner_coldblocks.sil

@@ -1,5 +1,5 @@
 // RUN: %empty-directory(%t)
-// RUN: %target-sil-opt -enable-sil-verify-all %s -inline -sil-remarks=sil-inliner -o %t.sil 2>&1 | %FileCheck -check-prefix=REMARKS_PASSED %s
+// RUN: %target-sil-opt -enable-sil-verify-all %s -compute-side-effects -inline -sil-remarks=sil-inliner -o %t.sil 2>&1 | %FileCheck -check-prefix=REMARKS_PASSED %s
 // RUN: %FileCheck %s < %t.sil
 // RUN: %target-sil-opt -enable-sil-verify-all %s -inline -sil-remarks-missed=sil-inliner -o /dev/null 2>&1 | %FileCheck -check-prefix=REMARKS_MISSED %s
 // RUN: %target-sil-opt -enable-sil-verify-all %s -inline -save-optimization-record-path %t.yaml -o /dev/null 2>&1 | %FileCheck -allow-empty -check-prefix=NO_REMARKS %s
@@ -9,6 +9,8 @@
 // REMARKS_MISSED-NOT: remark:
 // NO_REMARKS-NOT: remark:
 
+// REQUIRES: swift_in_compiler
+
 sil_stage canonical
 
 import Builtin
@@ -207,7 +209,7 @@ bb0:
   // YAML-NEXT:   - Callee:          '"update_global"'
   // YAML-NEXT:     DebugLoc:
   // YAML:          File:            {{.*}}inliner_coldblocks.sil
-  // YAML:            Line:            20
+  // YAML:            Line:            22
   // YAML:            Column:          6
   // YAML-NEXT:   - String:          ' inlined into '
   // YAML-NEXT:   - Caller:          '"regular_large_callee"'

test/SILOptimizer/side_effects.sil

@@ -655,7 +655,7 @@ bb0(%0 : $*X):
 }
 
 // CHECK-LABEL: sil @call_noreturn
-// CHECK-NEXT:  [global: ]
+// CHECK-NEXT:  [global: read]
 // CHECK-NEXT:  {{^[^[]}}
 sil @call_noreturn : $@convention(thin) () -> () {
 bb0:
@@ -1113,3 +1113,22 @@ bb0(%0 : $@callee_guaranteed () -> ()):
   return %1 : $Builtin.Int1
 }
 
+// CHECK-LABEL: sil @test_enum_and_p2a
+// CHECK-NEXT:  [%0: write v**.c*.v**]
+// CHECK-NEXT:  [global: write,deinit_barrier]
+// CHECK-NEXT:  {{^[^[]}}
+sil @test_enum_and_p2a : $@convention(thin) (Optional<UnsafeMutablePointer<Int>>, Int) -> () {
+bb0(%0 : $Optional<UnsafeMutablePointer<Int>>, %1 : $Int):
+  switch_enum %0 : $Optional<UnsafeMutablePointer<Int>>, case #Optional.some!enumelt: bb1, case #Optional.none!enumelt: bb2
+bb1(%2 : $UnsafeMutablePointer<Int>):
+  %3 = struct_extract %2 : $UnsafeMutablePointer<Int>, #UnsafeMutablePointer._rawValue
+  %4 = pointer_to_address %3 : $Builtin.RawPointer to [strict] $*Int
+  store %1 to %4 : $*Int
+  br bb3
+bb2:
+  br bb3
+bb3:
+  %r = tuple()
+  return %r : $()
+}
+