Merge pull request #69199 from eeckstein/fix-compile-time

Optimizer: better handling of the complexity budget in redundant-load-elimination and dead-store-elimination

Author: eeckstein

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/DeadStoreElimination.swift

@@ -53,6 +53,10 @@ import SIL
 let deadStoreElimination = FunctionPass(name: "dead-store-elimination") {
   (function: Function, context: FunctionPassContext) in
 
+  // Avoid quadratic complexity by limiting the number of visited instructions.
+  // This limit is sufficient for most "real-world" functions, by far.
+  var complexityBudget = 10_000
+
   for block in function.blocks {
 
     // We cannot use for-in iteration here because if the store is split, the new
@@ -63,26 +67,26 @@ let deadStoreElimination = FunctionPass(name: "dead-store-elimination") {
         if !context.continueWithNextSubpassRun(for: store) {
           return
         }
-        tryEliminate(store: store, context)
+        tryEliminate(store: store, complexityBudget: &complexityBudget, context)
       }
       inst = i.next
     }
   }
 }
 
-private func tryEliminate(store: StoreInst, _ context: FunctionPassContext) {
+private func tryEliminate(store: StoreInst, complexityBudget: inout Int, _ context: FunctionPassContext) {
   if !store.hasValidOwnershipForDeadStoreElimination {
     return
   }
 
-  switch store.isDead(context) {
+  switch store.isDead(complexityBudget: &complexityBudget, context) {
     case .alive:
       break
     case .dead:
       context.erase(instruction: store)
     case .maybePartiallyDead(let subPath):
       // Check if the a partial store would really be dead to avoid unnecessary splitting.
-      switch store.isDead(at: subPath, context) {
+      switch store.isDead(at: subPath, complexityBudget: &complexityBudget, context) {
         case .alive, .maybePartiallyDead:
           break
         case .dead:
@@ -109,15 +113,15 @@ private extension StoreInst {
     }
   }
 
-  func isDead( _ context: FunctionPassContext) -> DataflowResult {
-    return isDead(at: destination.accessPath, context)
+  func isDead(complexityBudget: inout Int, _ context: FunctionPassContext) -> DataflowResult {
+    return isDead(at: destination.accessPath, complexityBudget: &complexityBudget, context)
   }
 
-  func isDead(at accessPath: AccessPath, _ context: FunctionPassContext) -> DataflowResult {
+  func isDead(at accessPath: AccessPath, complexityBudget: inout Int, _ context: FunctionPassContext) -> DataflowResult {
     var scanner = InstructionScanner(storePath: accessPath, storeAddress: self.destination, context.aliasAnalysis)
     let storageDefBlock = accessPath.base.reference?.referenceRoot.parentBlock
 
-    switch scanner.scan(instructions: InstructionList(first: self.next)) {
+    switch scanner.scan(instructions: InstructionList(first: self.next), complexityBudget: &complexityBudget) {
     case .dead:
       return .dead
 
@@ -145,7 +149,7 @@ private extension StoreInst {
         if let storageDefBlock = storageDefBlock, block == storageDefBlock {
           return DataflowResult(aliveWith: scanner.potentiallyDeadSubpath)
         }
-        switch scanner.scan(instructions: block.instructions) {
+        switch scanner.scan(instructions: block.instructions, complexityBudget: &complexityBudget) {
         case .transparent:
           worklist.pushIfNotVisited(contentsOf: block.successors)
         case .dead:
@@ -177,10 +181,6 @@ private struct InstructionScanner {
 
   private(set) var potentiallyDeadSubpath: AccessPath? = nil
 
-  // Avoid quadratic complexity by limiting the number of visited instructions for each store.
-  // The limit of 1000 instructions is not reached by far in "real-world" functions.
-  private var budget = 1000
-
   init(storePath: AccessPath, storeAddress: Value, _ aliasAnalysis: AliasAnalysis) {
     self.storePath = storePath
     self.storeAddress = storeAddress
@@ -193,7 +193,7 @@ private struct InstructionScanner {
     case transparent
   }
 
-  mutating func scan(instructions: InstructionList) -> Result {
+  mutating func scan(instructions: InstructionList, complexityBudget: inout Int) -> Result {
     for inst in instructions {
       switch inst {
       case let successiveStore as StoreInst:
@@ -226,8 +226,8 @@ private struct InstructionScanner {
         }
         fallthrough
       default:
-        budget -= 1
-        if budget == 0 {
+        complexityBudget -= 1
+        if complexityBudget <= 0 {
           return .alive
         }
         if inst.mayRead(fromAddress: storeAddress, aliasAnalysis) {

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/RedundantLoadElimination.swift

@@ -74,6 +74,11 @@ let earlyRedundantLoadElimination = FunctionPass(name: "early-redundant-load-eli
 }
 
 private func eliminateRedundantLoads(in function: Function, ignoreArrays: Bool, _ context: FunctionPassContext) {
+
+  // Avoid quadratic complexity by limiting the number of visited instructions.
+  // This limit is sufficient for most "real-world" functions, by far.
+  var complexityBudget = 50_000
+
   for block in function.blocks.reversed() {
 
     // We cannot use for-in iteration here because if the load is split, the new
@@ -89,21 +94,21 @@ private func eliminateRedundantLoads(in function: Function, ignoreArrays: Bool,
         if ignoreArrays && load.type.isNominal && load.type.nominal == context.swiftArrayDecl {
           continue
         }
-        tryEliminate(load: load, context)
+        tryEliminate(load: load, complexityBudget: &complexityBudget, context)
       }
     }
   }
 }
 
-private func tryEliminate(load: LoadInst, _ context: FunctionPassContext) {
-  switch load.isRedundant(context) {
+private func tryEliminate(load: LoadInst, complexityBudget: inout Int, _ context: FunctionPassContext) {
+  switch load.isRedundant(complexityBudget: &complexityBudget, context) {
   case .notRedundant:
     break
   case .redundant(let availableValues):
     replace(load: load, with: availableValues, context)
   case .maybePartiallyRedundant(let subPath):
     // Check if the a partial load would really be redundant to avoid unnecessary splitting.
-    switch load.isRedundant(at: subPath, context) {
+    switch load.isRedundant(at: subPath, complexityBudget: &complexityBudget, context) {
       case .notRedundant, .maybePartiallyRedundant:
         break
       case .redundant:
@@ -130,25 +135,29 @@ private extension LoadInst {
     }
   }
 
-  func isRedundant(_ context: FunctionPassContext) -> DataflowResult {
-    return isRedundant(at: address.accessPath, context)
+  func isRedundant(complexityBudget: inout Int, _ context: FunctionPassContext) -> DataflowResult {
+    return isRedundant(at: address.accessPath, complexityBudget: &complexityBudget, context)
   }
 
-  func isRedundant(at accessPath: AccessPath, _ context: FunctionPassContext) -> DataflowResult {
+  func isRedundant(at accessPath: AccessPath, complexityBudget: inout Int, _ context: FunctionPassContext) -> DataflowResult {
     var scanner = InstructionScanner(load: self, accessPath: accessPath, context.aliasAnalysis)
 
-    switch scanner.scan(instructions: ReverseInstructionList(first: self.previous), in: parentBlock) {
+    switch scanner.scan(instructions: ReverseInstructionList(first: self.previous),
+                        in: parentBlock,
+                        complexityBudget: &complexityBudget)
+    {
     case .overwritten:
       return DataflowResult(notRedundantWith: scanner.potentiallyRedundantSubpath)
     case .available:
       return .redundant(scanner.availableValues)
     case .transparent:
-      return self.isRedundantInPredecessorBlocks(scanner: &scanner, context)
+      return self.isRedundantInPredecessorBlocks(scanner: &scanner, complexityBudget: &complexityBudget, context)
     }
   }
 
   private func isRedundantInPredecessorBlocks(
     scanner: inout InstructionScanner,
+    complexityBudget: inout Int,
     _ context: FunctionPassContext
   ) -> DataflowResult {
 
@@ -157,7 +166,10 @@ private extension LoadInst {
     liferange.pushPredecessors(of: self.parentBlock)
 
     while let block = liferange.pop() {
-      switch scanner.scan(instructions: block.instructions.reversed(), in: block) {
+      switch scanner.scan(instructions: block.instructions.reversed(),
+                          in: block,
+                          complexityBudget: &complexityBudget)
+      {
       case .overwritten:
         return DataflowResult(notRedundantWith: scanner.potentiallyRedundantSubpath)
       case .available:
@@ -402,10 +414,6 @@ private struct InstructionScanner {
   private(set) var potentiallyRedundantSubpath: AccessPath? = nil
   private(set) var availableValues = Array<AvailableValue>()
 
-  // Avoid quadratic complexity by limiting the number of visited instructions for each store.
-  // The limit of 1000 instructions is not reached by far in "real-world" functions.
-  private var budget = 1000
-
   init(load: LoadInst, accessPath: AccessPath, _ aliasAnalysis: AliasAnalysis) {
     self.load = load
     self.accessPath = accessPath
@@ -419,8 +427,16 @@ private struct InstructionScanner {
     case transparent
   }
 
-  mutating func scan(instructions: ReverseInstructionList, in block: BasicBlock) -> ScanResult {
+  mutating func scan(instructions: ReverseInstructionList,
+                     in block: BasicBlock,
+                     complexityBudget: inout Int) -> ScanResult
+  {
     for inst in instructions {
+      complexityBudget -= 1
+      if complexityBudget <= 0 {
+        return .overwritten
+      }
+
       switch visit(instruction: inst) {
         case .available:   return .available
         case .overwritten: return .overwritten
@@ -490,10 +506,6 @@ private struct InstructionScanner {
     default:
       break
     }
-    budget -= 1
-    if budget == 0 {
-      return .overwritten
-    }
     if load.loadOwnership == .take {
       // In case of `take`, don't allow reading instructions in the liferange.
       // Otherwise we cannot shrink the memory liferange afterwards.

validation-test/SILOptimizer/rle_dse_complexity.swift

@@ -0,0 +1,26 @@
+
+// The compiler should finish in about 5 seconds. To give some slack,
+// specify a timeout of 60 seconds
+// If the compiler needs more than 60 seconds, there is probably a real problem.
+// So please don't just increase the timeout in case this test fails.
+
+// RUN: %{python} %S/../../test/Inputs/timeout.py 60 %target-swift-frontend -O -parse-as-library -sil-verify-none -emit-sil %s | %FileCheck %s
+
+// REQUIRES: swift_stdlib_no_asserts,optimized_stdlib
+// REQUIRES: CPU=arm64 || CPU=x86_64 || CPU=aarch64
+// REQUIRES: OS=macosx
+
+import Darwin
+
+// Check that redundant-load-elimination and dead-store-elimination don't take
+// extermely long when optimizing statfs, which contains a 1023-element tuple.
+
+// CHECK-LABEL: test_rle_dse_compile_time
+public func test_rle_dse_compile_time(_ s: statfs) {
+  withUnsafePointer(to: s.f_mntonname) {
+    $0.withMemoryRebound(to: UInt8.self, capacity: Int(MAXPATHLEN)) { (str) in
+      print(str)
+    }
+  }
+}
+