Merge pull request #34968 from gottesmm/pr-50e8ac8f699f7e3ef7096b4b56510ffc897669e5

[mandatory-combine] Add a canonicalization impl and prepare to use mandatory-combine to test ownership RAUW code

Author: gottesmm

lib/SILOptimizer/Mandatory/MandatoryCombine.cpp

@@ -31,6 +31,7 @@
 #include "swift/SIL/SILVisitor.h"
 #include "swift/SILOptimizer/PassManager/Passes.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
+#include "swift/SILOptimizer/Utils/CanonicalizeInstruction.h"
 #include "swift/SILOptimizer/Utils/InstOptUtils.h"
 #include "swift/SILOptimizer/Utils/StackNesting.h"
 #include "llvm/ADT/STLExtras.h"
@@ -53,6 +54,54 @@ static bool areAllValuesTrivial(Values values, SILFunction &function) {
   });
 }
 
+//===----------------------------------------------------------------------===//
+//      CanonicalizeInstruction subclass for use in Mandatory Combiner.
+//===----------------------------------------------------------------------===//
+
+namespace {
+
+class MandatoryCombineCanonicalize final : CanonicalizeInstruction {
+public:
+  using Worklist = SmallSILInstructionWorklist<256>;
+
+private:
+  Worklist &worklist;
+  bool changed = false;
+
+public:
+  MandatoryCombineCanonicalize(Worklist &worklist)
+      : CanonicalizeInstruction(DEBUG_TYPE), worklist(worklist) {}
+
+  void notifyNewInstruction(SILInstruction *inst) override {
+    worklist.add(inst);
+    worklist.addUsersOfAllResultsToWorklist(inst);
+    changed = true;
+  }
+
+  // Just delete the given 'inst' and record its operands. The callback isn't
+  // allowed to mutate any other instructions.
+  void killInstruction(SILInstruction *inst) override {
+    worklist.eraseSingleInstFromFunction(*inst,
+                                         /*AddOperandsToWorklist*/ true);
+    changed = true;
+  }
+
+  void notifyHasNewUsers(SILValue value) override {
+    if (worklist.size() < 10000) {
+      worklist.addUsersToWorklist(value);
+    }
+    changed = true;
+  }
+
+  bool tryCanonicalize(SILInstruction *inst) {
+    changed = false;
+    canonicalize(inst);
+    return changed;
+  }
+};
+
+} // anonymous namespace
+
 //===----------------------------------------------------------------------===//
 //                        MandatoryCombiner Interface
 //===----------------------------------------------------------------------===//
@@ -137,6 +186,13 @@ class MandatoryCombiner final
 //               MandatoryCombiner Non-Visitor Utility Methods
 //===----------------------------------------------------------------------===//
 
+static llvm::cl::opt<bool> EnableCanonicalizationAndTrivialDCE(
+    "sil-mandatory-combine-enable-canon-and-simple-dce", llvm::cl::Hidden,
+    llvm::cl::init(false),
+    llvm::cl::desc("An option for compiler developers that cause the Mandatory "
+                   "Combiner to be more aggressive at eliminating trivially "
+                   "dead code and canonicalizing SIL"));
+
 void MandatoryCombiner::addReachableCodeToWorklist(SILFunction &function) {
   SmallVector<SILBasicBlock *, 32> blockWorklist;
   SmallPtrSet<SILBasicBlock *, 32> blockAlreadyAddedToWorklist;
@@ -148,6 +204,8 @@ void MandatoryCombiner::addReachableCodeToWorklist(SILFunction &function) {
     blockAlreadyAddedToWorklist.insert(firstBlock);
   }
 
+  bool compilingWithOptimization = function.getEffectiveOptimizationMode() !=
+                                   OptimizationMode::NoOptimization;
   while (!blockWorklist.empty()) {
     auto *block = blockWorklist.pop_back_val();
 
@@ -156,6 +214,12 @@ void MandatoryCombiner::addReachableCodeToWorklist(SILFunction &function) {
       ++iterator;
 
       if (isInstructionTriviallyDead(instruction)) {
+        if (EnableCanonicalizationAndTrivialDCE) {
+          if (compilingWithOptimization) {
+            instruction->replaceAllUsesOfAllResultsWithUndef();
+            instruction->eraseFromParent();
+          }
+        }
         continue;
       }
 
@@ -177,13 +241,32 @@ bool MandatoryCombiner::doOneIteration(SILFunction &function,
   madeChange = false;
 
   addReachableCodeToWorklist(function);
+  MandatoryCombineCanonicalize mcCanonicialize(worklist);
+
+  bool compilingWithOptimization = function.getEffectiveOptimizationMode() !=
+                                   OptimizationMode::NoOptimization;
 
   while (!worklist.isEmpty()) {
     auto *instruction = worklist.pop_back_val();
     if (instruction == nullptr) {
       continue;
     }
 
+    if (EnableCanonicalizationAndTrivialDCE) {
+      if (compilingWithOptimization) {
+        if (isInstructionTriviallyDead(instruction)) {
+          worklist.eraseInstFromFunction(*instruction);
+          madeChange = true;
+          continue;
+        }
+      }
+
+      if (mcCanonicialize.tryCanonicalize(instruction)) {
+        madeChange = true;
+        continue;
+      }
+    }
+
 #ifndef NDEBUG
     std::string instructionDescription;
 #endif

lib/SILOptimizer/Utils/InstOptUtils.cpp

@@ -138,6 +138,12 @@ swift::createDecrementBefore(SILValue ptr, SILInstruction *insertPt) {
   return builder.createReleaseValue(loc, ptr, builder.getDefaultAtomicity());
 }
 
+static bool isOSSAEndScopeWithNoneOperand(SILInstruction *i) {
+  if (!isa<EndBorrowInst>(i) && !isa<DestroyValueInst>(i))
+    return false;
+  return i->getOperand(0).getOwnershipKind() == OwnershipKind::None;
+}
+
 /// Perform a fast local check to see if the instruction is dead.
 ///
 /// This routine only examines the state of the instruction at hand.
@@ -178,6 +184,15 @@ bool swift::isInstructionTriviallyDead(SILInstruction *inst) {
   if (isa<UncheckedTakeEnumDataAddrInst>(inst))
     return true;
 
+  // An ossa end scope instruction is trivially dead if its operand has
+  // OwnershipKind::None. This can occur after CFG simplification in the
+  // presence of non-payloaded or trivial payload cases of non-trivial enums.
+  //
+  // Examples of ossa end_scope instructions: end_borrow, destroy_value.
+  if (inst->getFunction()->hasOwnership() &&
+      isOSSAEndScopeWithNoneOperand(inst))
+    return true;
+
   if (!inst->mayHaveSideEffects())
     return true;
 

test/SILOptimizer/mandatory_combiner.sil

@@ -75,6 +75,12 @@ sil @myint_from_myint_and_proto : $@convention(thin) (MyInt, @guaranteed { var P
 
 sil @myint_from_proto_and_myint : $@convention(thin) (@guaranteed { var Proto }, MyInt) -> MyInt
 
+// Optional support
+enum FakeOptional<T> {
+case none
+case some(T)
+}
+
 ///////////
 // Tests //
 ///////////

test/SILOptimizer/mandatory_combiner_opt.sil

@@ -0,0 +1,108 @@
+// RUN: %target-sil-opt -mandatory-combine -sil-mandatory-combine-enable-canon-and-simple-dce %s | %FileCheck %s
+
+// Tests for when the mandatory combiner is running with optimizations
+// enabled. Only put tests here for functionality that only occurs when the
+// Mandatory Combiner runs in between the diagnostics/perf pipeline at -O,
+// -Osize.
+
+sil_stage canonical
+
+import Builtin
+
+// Trivial declarations
+
+struct MyInt {
+  var value: Builtin.Int64
+}
+
+// Generic declarations
+
+protocol Addable {
+  static var an: Self { get }
+}
+
+// Class declarations
+
+class Klass {
+  init()
+  deinit
+}
+
+// Existential declarations
+
+protocol Proto {
+  static var an: Proto { get }
+}
+
+// Trivial support
+
+sil @first_of_three_ints : $@convention(thin) (MyInt, MyInt, MyInt) -> MyInt
+
+sil @constant_zero : $@convention(thin) () -> MyInt 
+
+sil @identity_int : $@convention(thin) (MyInt) -> MyInt 
+
+// Generic support
+
+sil @first_of_three_addables : $@convention(thin) <A where A : Addable> (@in_guaranteed A, @guaranteed <τ_0_0 where τ_0_0 : Addable> { var τ_0_0 } <A>, @guaranteed <τ_0_0 where τ_0_0 : Addable> { var τ_0_0 } <A>) -> @
+out A
+
+// Class support
+
+sil [exact_self_class] @klass_alloc_init : $@convention(method) (@thick Klass.Type) -> @owned Klass
+
+// Klass.init()
+sil @klass_init : $@convention(method) (@owned Klass) -> @owned Klass
+// Klass.deinit
+sil @klass_deinit : $@convention(method) (@guaranteed Klass) -> @owned Builtin.NativeObject
+
+// Klass.__deallocating_deinit
+sil @klass_dealloc_deinit : $@convention(method) (@owned Klass) -> ()
+
+sil_vtable Klass {
+  #Klass.init!allocator: (Klass.Type) -> () -> Klass : @klass_alloc_init
+  #Klass.deinit!deallocator: @klass_dealloc_deinit
+}
+
+sil @first_of_three_klasses : $@convention(thin) (@guaranteed Klass, @guaranteed Klass, @guaranteed Klass) -> @owned Klass
+
+// Existential support
+
+sil @first_of_three_protos : $@convention(thin) (@in_guaranteed Proto, @guaranteed { var Proto }, @guaranteed { var Proto }) -> @out Proto
+
+sil @get_proto : $@convention(thin) () -> @out Proto
+
+// Mixed support
+
+sil @proto_from_proto_and_myint : $@convention(thin) (@in_guaranteed Proto, MyInt) -> @out Proto
+
+sil @myint_from_myint_and_proto : $@convention(thin) (MyInt, @guaranteed { var Proto }) -> MyInt
+
+sil @myint_from_proto_and_myint : $@convention(thin) (@guaranteed { var Proto }, MyInt) -> MyInt
+
+// Optional support
+enum FakeOptional<T> {
+case none
+case some(T)
+}
+
+///////////
+// Tests //
+///////////
+
+
+// CHECK-LABEL: sil [ossa] @eliminate_simple_arc_traffic : $@convention(thin) (@guaranteed Klass) -> () {
+// CHECK-NOT: copy_value
+// CHECK-NOT: destroy_value
+// CHECK-NOT: enum
+// CHECK-NOT: end_borrow
+// CHECK: } // end sil function 'eliminate_simple_arc_traffic'
+sil [ossa] @eliminate_simple_arc_traffic : $@convention(thin) (@guaranteed Klass) -> () {
+bb0(%0 : @guaranteed $Klass):
+  %1 = copy_value %0 : $Klass
+  destroy_value %1 : $Klass
+  %2 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
+  end_borrow %2 : $FakeOptional<Klass>
+  %9999 = tuple()
+  return %9999 : $()
+}

test/stdlib/Inputs/CommonArrayTests.gyb

@@ -5,6 +5,9 @@
 % #   - Suite -- an identifier for the test suite to append tests to.
 % #   - ArrayType -- the type being tested.
 
+// We use this global array to prevent ARC from eliminating temporary ARC
+// traffic in nonUniqueCode below. It is only ever assigned to.
+var globalArrayForNonUnique = ${ArrayType}<Int>()
 
 extension ${ArrayType} {
   typealias _BufferID = UnsafeRawPointer?
@@ -107,7 +110,7 @@ ${Suite}.test("${ArrayType}/appendNonUnique")
   x.reserveCapacity(10002)
   let capacity = x.capacity
   for _ in 1...100 {
-    let y = x
+    globalArrayForNonUnique = x
     x.append(1)
     expectTrue(x.capacity == capacity)
   }
@@ -120,7 +123,7 @@ ${Suite}.test("${ArrayType}/removeNonUnique")
     var x = ${ArrayType}<Int>(repeating: 27, count: 200)
     x.reserveCapacity(10002)
     for _ in 1...100 {
-      let y = x
+      globalArrayForNonUnique = x
       x.remove(at: 0)
       expectTrue(x.capacity < 1000)
     }
@@ -133,7 +136,7 @@ ${Suite}.test("${ArrayType}/mutateNonUnique")
   var x = ${ArrayType}<Int>(repeating: 27, count: 200)
   x.reserveCapacity(10002)
   for _ in 1...100 {
-    let y = x
+    globalArrayForNonUnique = x
     x[0] = 0
     expectTrue(x.capacity < 1000)
   }