Merge pull request #69002 from atrick/test-forwarding

ForwardingUtils.swift fixes and unit tests

Author: atrick

SwiftCompilerSources/Sources/Basic/Utils.swift

@@ -52,6 +52,19 @@ public extension NoReflectionChildren {
   var customMirror: Mirror { Mirror(self, children: []) }
 }
 
+public var standardError = CFileStream(fp: stderr)
+
+public struct CFileStream: TextOutputStream {
+  var fp: UnsafeMutablePointer<FILE>
+
+  public func write(_ string: String) {
+    fputs(string, fp)
+  }
+
+  public func flush() {
+    fflush(fp)
+  }
+}
 
 //===----------------------------------------------------------------------===//
 //                              StringRef

SwiftCompilerSources/Sources/Optimizer/Utilities/ForwardingUtils.swift

@@ -1,4 +1,4 @@
-//===--- OwnershipUtils.swift - Utilities for ownership -------------------===//
+//===--- ForwardingUtils.swift - Utilities for ownership forwarding -------===//
 //
 // This source file is part of the Swift.org open source project
 //
@@ -29,8 +29,6 @@ import SIL
 //
 // Note: Although move_value conceptually forwards an owned value, it also summarizes lifetime attributes; therefore, it is not formally a ForwardingInstruction.
 //
-// TODO: when phi lifetime flags are implemented, phis will introduce a lifetime in the same way as move_value.
-//
 // The lifetime introducer of a guaranteed value is the borrow introducer:
 //
 //   # lifetime introducer / borrow introducer
@@ -43,11 +41,25 @@ import SIL
 //
 // TODO: When a begin_borrow has no lifetime flags, it can be ignored as a lifetime introducer. In that case, an owned value may introduce guaranteed OSSA lifetimes.
 //
+// Forwarded lifetimes also extend through phis. In this case, however, there is no ForwardingInstruction.
+//
+//   # lifetime introducer
+//   %1 = apply             -+               -+
+//   ...                     | OSSA lifetime  |
+//   # phi operand           |                |
+//   br bbContinue(%1: $S)  -+                | forward-extended lifetime
+//                                            |
+//   bbContinue(%phi : $S): -+ OSSA lifetime  |
+//   ...                     |                |
+//   destroy_value %phi     -+               -+
+//
+// TODO: when phi lifetime flags are implemented, phis will introduce a lifetime in the same way as move_value.
+//
 // This walker is used to query basic lifetime attributes on values, such as "escaping" or "lexical". It must be precise for correctness and is performance critical.
 protocol ForwardingUseDefWalker {
   mutating func introducer(_ value: Value) -> WalkResult
 
-  // Minimally, check a ValueSet. This walker may traverse chains of aggregation and destructuring by default. Implementations may traverse phis.
+  // Minimally, check a ValueSet. This walker may traverse chains of aggregation and destructuring along with phis.
   mutating func needWalk(for value: Value) -> Bool
 
   mutating func walkUp(value: Value) -> WalkResult
@@ -58,10 +70,12 @@ extension ForwardingUseDefWalker {
     walkUpDefault(value: value)
   }
   mutating func walkUpDefault(value: Value) -> WalkResult {
-    if let inst = value.definingInstruction as? ForwardingInstruction
-    {
+    if let inst = value.forwardingInstruction {
       return walkUp(instruction: inst)
     }
+    if let phi = Phi(value) {
+      return walkUp(phi: phi)
+    }
     return introducer(value)
   }
   mutating func walkUp(instruction: ForwardingInstruction) -> WalkResult {
@@ -74,6 +88,16 @@ extension ForwardingUseDefWalker {
     }
     return .continueWalk
   }
+  mutating func walkUp(phi: Phi) -> WalkResult {
+    for operand in phi.incomingOperands {
+      if needWalk(for: operand.value) {
+        if walkUp(value: operand.value) == .abortWalk {
+          return .abortWalk
+        }
+      }
+    }
+    return .continueWalk
+  }
 }
 
 // This conveniently gathers all forward introducers and deinitializes visitedValues before the caller has a chance to recurse.
@@ -103,14 +127,34 @@ private struct GatherLifetimeIntroducers : ForwardingUseDefWalker {
   }
 }
 
+enum ForwardingUseResult: CustomStringConvertible {
+  case operand(Operand)
+  case deadValue(Value)
+
+  var description: String {
+    switch self {
+    case .operand(let operand):
+      return operand.description
+    case .deadValue(let deadValue):
+      return "dead value: " + deadValue.description
+    }
+  }
+}
+
 // Visit all the uses in a forward-extended lifetime (LifetimeIntroducer -> Operand).
 protocol ForwardingDefUseWalker {
   // Minimally, check a ValueSet. This walker may traverse chains of aggregation and destructuring by default. Implementations may handle phis.
   mutating func needWalk(for value: Value) -> Bool
 
-  mutating func leafUse(_ operand: Operand) -> WalkResult
+  // Report any initial or forwarded  with no uses. Only relevant for
+  // guaranteed values or incomplete OSSA. This could be a dead
+  // instruction, a terminator in which the result is dead on one
+  // path, or a dead phi.
+  mutating func deadValue(_: Value) -> WalkResult
+
+  mutating func leafUse(_: Operand) -> WalkResult
 
-  mutating func walkDownUses(of value: Value) -> WalkResult
+  mutating func walkDownUses(of: Value) -> WalkResult
 
   mutating func walkDown(operand: Operand) -> WalkResult
 }
@@ -123,10 +167,15 @@ extension ForwardingDefUseWalker {
   mutating func walkDownUsesDefault(of value: Value) -> WalkResult {
     if !needWalk(for: value) { return .continueWalk }
 
+    var hasUse = false
     for operand in value.uses where !operand.isTypeDependent {
       if walkDown(operand: operand) == .abortWalk {
         return .abortWalk
       }
+      hasUse = true
+    }
+    if !hasUse {
+      deadValue(value)
     }
     return .continueWalk
   }
@@ -139,6 +188,9 @@ extension ForwardingDefUseWalker {
     if let inst = operand.instruction as? ForwardingInstruction {
       return walkDownAllResults(of: inst)
     }
+    if let phi = Phi(using: operand) {
+      return walkDownUses(of: phi.value)
+    }
     return leafUse(operand)
   }
 
@@ -156,9 +208,8 @@ extension ForwardingDefUseWalker {
 // This conveniently allows a closure to be called for each leaf use of a forward-extended lifetime. It should be called on a forward introducer provided by ForwardingDefUseWalker.introducer() or gatherLifetimeIntroducers().
 //
 // TODO: make the visitor non-escaping once Swift supports stored non-escaping closues.
-func visitForwardedUses(introducer: Value,
-  visitor: @escaping (Operand) -> WalkResult,
-  _ context: Context)
+func visitForwardedUses(introducer: Value, _ context: Context,
+  visitor: @escaping (ForwardingUseResult) -> WalkResult)
 -> WalkResult {
   var useVisitor = VisitForwardedUses(visitor: visitor, context)
   defer { useVisitor.visitedValues.deinitialize() }
@@ -167,9 +218,10 @@ func visitForwardedUses(introducer: Value,
 
 private struct VisitForwardedUses : ForwardingDefUseWalker {
   var visitedValues: ValueSet
-  var visitor: (Operand) -> WalkResult
+  var visitor: (ForwardingUseResult) -> WalkResult
 
-  init(visitor: @escaping (Operand) -> WalkResult, _ context: Context) {
+  init(visitor: @escaping (ForwardingUseResult) -> WalkResult,
+    _ context: Context) {
     self.visitedValues = ValueSet(context)
     self.visitor = visitor
   }
@@ -179,6 +231,27 @@ private struct VisitForwardedUses : ForwardingDefUseWalker {
   }
 
   mutating func leafUse(_ operand: Operand) -> WalkResult {
-    return visitor(operand)
+    return visitor(.operand(operand))
+  }
+
+  mutating func deadValue(_ value: Value) -> WalkResult {
+    return visitor(.deadValue(value))
+  }
+}
+
+let forwardingUseDefTest = FunctionTest("forwarding_use_def_test") {
+  function, arguments, context in
+  let value = arguments.takeValue()
+  for introducer in gatherLifetimeIntroducers(for: value, context) {
+    print("INTRODUCER: \(introducer)")
+  }
+}
+
+let forwardingDefUseTest = FunctionTest("forwarding_def_use_test") {
+  function, arguments, context in
+  let value = arguments.takeValue()
+  _ = visitForwardedUses(introducer: value, context) { operand in
+    print("USE: \(operand)")
+    return .continueWalk
   }
 }

SwiftCompilerSources/Sources/Optimizer/Utilities/Test.swift

@@ -16,8 +16,8 @@
 //       FunctionTest("my_new_test") { function, arguments, context in
 //       }
 // - In SwiftCompilerSources/Sources/SIL/Test.swift's registerOptimizerTests
-//   function, register the new test:
-//       registerFunctionTest(myNewTest)
+//   function, add a new argument to the variadic function:
+//       registerFunctionTests(..., myNewTest)
 //
 //===----------------------------------------------------------------------===//
 //
@@ -100,7 +100,7 @@ struct FunctionTest {
   let name: String
   let invocation: FunctionTestInvocation
 
-  public init(name: String, invocation: @escaping FunctionTestInvocation) {
+  public init(_ name: String, invocation: @escaping FunctionTestInvocation) {
     self.name = name
     self.invocation = invocation
   }
@@ -135,12 +135,19 @@ extension BridgedTestArguments {
 /// Registration of each test in the SIL module.
 public func registerOptimizerTests() {
   // Register each test.
-  registerFunctionTest(parseTestSpecificationTest)
+  registerFunctionTests(
+      parseTestSpecificationTest,
+      forwardingUseDefTest,
+      forwardingDefUseTest
+    )
 
   // Finally register the thunk they all call through.
   registerFunctionTestThunk(functionTestThunk)
 }
 
+private func registerFunctionTests(_ tests: FunctionTest...) {
+  tests.forEach { registerFunctionTest($0) }
+}
 
 private func registerFunctionTest(_ test: FunctionTest) {
   test.name._withBridgedStringRef { ref in
@@ -202,7 +209,7 @@ private func castToInvocation(fromOpaquePointer erasedInvocation: UnsafeMutableR
 //   - its type
 //   - something to identify the instance (mostly this means calling dump)
 let parseTestSpecificationTest = 
-FunctionTest(name: "test_specification_parsing") { function, arguments, context in 
+FunctionTest("test_specification_parsing") { function, arguments, context in 
   let expectedFields = arguments.takeString()
   for expectedField in expectedFields.string {
     switch expectedField {

SwiftCompilerSources/Sources/SIL/Argument.swift

@@ -64,7 +64,19 @@ public struct Phi {
     guard term is BranchInst || term is CondBranchInst else { return nil }
     self.value = argument
   }
-  
+
+  public init?(using operand: Operand) {
+    switch operand.instruction {
+    case let br as BranchInst:
+      self.init(br.getArgument(for: operand))
+    case let condBr as CondBranchInst:
+      guard let arg = condBr.getArgument(for: operand) else { return nil }
+      self.init(arg)
+    default:
+      return nil
+    }
+  }
+
   public var predecessors: PredecessorList {
     return value.parentBlock.predecessors
   }

SwiftCompilerSources/Sources/SIL/ForwardingInstruction.swift

@@ -79,8 +79,8 @@ extension ForwardingInstruction {
   // See ForwardingOperation::getSingleForwardingOperand().
   public var singleForwardedOperand: Operand? {
     let definedOps = self.definedOperands
-    assert(definedOps.count == 1);
-    return definedOps[0];
+    assert(definedOps.count == 1, "expected single operand for forwarding")
+    return definedOps[0]
   }
 }
 
@@ -117,19 +117,19 @@ extension DifferentiableFunctionInst {
 // Instructions with a singleForwardedOperand and additional operands.
 
 extension MarkDependenceInst {
-  public var singleForwardedOperand: Operand {
+  public var singleForwardedOperand: Operand? {
     return valueOperand
   }
 }
 
 extension RefToBridgeObjectInst {
-  public var singleForwardedOperand: Operand {
+  public var singleForwardedOperand: Operand? {
     return convertedOperand
   }
 }
 
 extension TuplePackExtractInst {
-  public var singleForwardedOperand: Operand {
+  public var singleForwardedOperand: Operand? {
     return tupleOperand
   }
 }

include/swift/SIL/Test.h

@@ -167,7 +167,7 @@ class FunctionTest final {
   template <typename Analysis, typename Transform = SILFunctionTransform>
   Analysis *getAnalysis();
 
-  SwiftPassInvocation *getInvocation();
+  SwiftPassInvocation *getSwiftPassInvocation();
 
 //===----------------------------------------------------------------------===//
 //=== MARK: Implementation Details                                         ===
@@ -247,7 +247,7 @@ class FunctionTest final {
   struct Dependencies {
     virtual DominanceInfo *getDominanceInfo() = 0;
     virtual SILPassManager *getPassManager() = 0;
-    virtual SwiftPassInvocation *getInvocation() = 0;
+    virtual SwiftPassInvocation *getSwiftPassInvocation() = 0;
     virtual ~Dependencies(){};
   };
 

include/swift/SILOptimizer/PassManager/PassManager.h

@@ -99,6 +99,10 @@ class SwiftPassInvocation {
                          SILCombiner *silCombiner) :
     passManager(passManager), function(function), silCombiner(silCombiner) {}
 
+  SwiftPassInvocation(SILPassManager *passManager, SILTransform *transform,
+                      SILFunction *function) :
+    passManager(passManager), transform(transform), function(function) {}
+
   SwiftPassInvocation(SILPassManager *passManager) :
     passManager(passManager) {}
 

lib/SIL/Utils/Test.cpp

@@ -100,7 +100,7 @@ void FunctionTest::run(SILFunction &function, Arguments &arguments,
     llvm::outs().flush();
     auto *fn = invocation.get<NativeSwiftInvocation>();
     Registry::get().getFunctionTestThunk()(fn, {&function}, {&arguments},
-                                           {getInvocation()});
+                                           {getSwiftPassInvocation()});
   }
   this->pass = nullptr;
   this->function = nullptr;
@@ -115,6 +115,6 @@ SILPassManager *FunctionTest::getPassManager() {
   return dependencies->getPassManager();
 }
 
-SwiftPassInvocation *FunctionTest::getInvocation() {
-  return dependencies->getInvocation();
+SwiftPassInvocation *FunctionTest::getSwiftPassInvocation() {
+  return dependencies->getSwiftPassInvocation();
 }

lib/SILOptimizer/UtilityPasses/TestRunner.cpp

@@ -60,14 +60,17 @@ class TestRunner : public SILFunctionTransform {
       : public test::FunctionTest::Dependencies {
     TestRunner *pass;
     SILFunction *function;
-    SwiftPassInvocation invocation;
+    SwiftPassInvocation swiftPassInvocation;
     FunctionTestDependenciesImpl(TestRunner *pass, SILFunction *function)
-        : pass(pass), function(function), invocation(pass->getPassManager()) {}
+        : pass(pass), function(function),
+          swiftPassInvocation(pass->getPassManager(), pass, function) {}
     DominanceInfo *getDominanceInfo() override {
       auto *dominanceAnalysis = pass->getAnalysis<DominanceAnalysis>();
       return dominanceAnalysis->get(function);
     }
-    SwiftPassInvocation *getInvocation() override { return &invocation; }
+    SwiftPassInvocation *getSwiftPassInvocation() override {
+      return &swiftPassInvocation;
+    }
     SILPassManager *getPassManager() override { return pass->getPassManager(); }
     ~FunctionTestDependenciesImpl() {}
   };