[sil-cloner] Always call doPreProcess when visiting SILInstructions.

It is important to call doPreProcess to correctly setup the available opened archetypes which were referenced from the original instruction being copied.

This fixes a concrete bug in LoopRotate optimization and potential bugs related to cloning.

rdar://27659420

Author: swiftix

include/swift/SIL/SILCloner.h

@@ -66,6 +66,14 @@ class SILCloner : protected SILVisitor<ImplClass> {
   SILBuilder &getBuilder() { return Builder; }
 
 protected:
+  void beforeVisit(ValueBase *V) {
+    if (auto I = dyn_cast<SILInstruction>(V)) {
+      // Update the set of available opened archetypes with the opened
+      // archetypes used by the current instruction.
+     doPreProcess(I);
+    }
+  }
+
 #define VALUE(CLASS, PARENT) \
   void visit##CLASS(CLASS *I) {                                       \
     llvm_unreachable("SILCloner visiting non-instruction?");          \
@@ -380,9 +388,6 @@ SILCloner<ImplClass>::visitSILBasicBlock(SILBasicBlock* BB) {
   SILFunction &F = getBuilder().getFunction();
   // Iterate over and visit all instructions other than the terminator to clone.
   for (auto I = BB->begin(), E = --BB->end(); I != E; ++I) {
-    // Update the set of available opened archetypes with the opened archetypes
-    // used by the current instruction.
-    doPreProcess(&*I);
     asImpl().visit(&*I);
   }
   // Iterate over successors to do the depth-first search.

include/swift/SIL/SILVisitor.h

@@ -31,7 +31,15 @@ class SILVisitor {
 public:
   ImplClass &asImpl() { return static_cast<ImplClass &>(*this); }
 
+  // Peform any required pre-processing before visiting.
+  // Sub-classes can override it to provide their custom
+  // pre-processing steps.
+  void beforeVisit(ValueBase *V) {
+  }
+
   ValueRetTy visit(ValueBase *V) {
+    asImpl().beforeVisit(V);
+
     switch (V->getKind()) {
 #define VALUE(CLASS, PARENT)                                \
   case ValueKind::CLASS:                                    \

test/SILOptimizer/looprotate.sil

@@ -21,15 +21,22 @@ import Swift
 // CHECK:   [[STRUCT:%.*]] = struct $Int32 ({{%.*}} : $Builtin.Int32)
 // CHECK:   return [[STRUCT]] : $Int32
 
+protocol P {
+  func boo() -> Int64
+}
+
 class Bar {
+  func boo() -> Int64
   func foo()
   @objc func foo_objc()
 }
 
 sil @_TFC4main3Bar3foofS0_FT_T_ : $@convention(method) (@guaranteed Bar) -> ()
+sil @_TFC4main3Bar3boofS0_FT_T_ : $@convention(method) (@guaranteed Bar) -> Int64
 sil @_TFC4main3Bar3foo_objcfS0_FT_T_ : $@convention(objc_method) (Bar) -> ()
 
 sil_vtable Bar {
+  #Bar.boo!1: _TFC4main3Bar3boofS0_FT_T_
   #Bar.foo!1: _TFC4main3Bar3foofS0_FT_T_
   #Bar.foo_objc!1: _TFC4main3Bar3foofS0_FT_T_
 }
@@ -104,6 +111,45 @@ bb3 (%23 : $Builtin.Int32) :
   return %24 : $Int32
 }
 
+// The following function used to crash the compiler, because loop rotate
+// could not properly handle the reference from witness_method to the
+// archetype opened by open_existential_addr 
+// CHECK-LABEL: sil @looprotate_with_opened_archetype
+// CHECK: return
+sil @looprotate_with_opened_archetype : $@convention(thin) (Int32, @in P) -> Int32 {
+bb0(%0 : $Int32, %25: $*P):
+  %1 = struct_extract %0 : $Int32, #Int32._value
+  %2 = integer_literal $Builtin.Int32, 0
+  %30 = alloc_box $Bool
+  %30a = project_box %30 : $@box Bool
+  %40 = open_existential_addr %25 : $*P to $*@opened("C22498FA-CABF-11E5-B9A9-685B35C48C83") P
+  br bb1(%1 : $Builtin.Int32, %2 : $Builtin.Int32, %25: $*P, %30 : $@box Bool, %30a : $*Bool)
+
+bb1(%4 : $Builtin.Int32, %5 : $Builtin.Int32, %26: $*P, %31 : $@box Bool, %32 : $*Bool):
+  %111 = witness_method $@opened("C22498FA-CABF-11E5-B9A9-685B35C48C83") P, #P.boo!1, %40 : $*@opened("C22498FA-CABF-11E5-B9A9-685B35C48C83") P : $@convention(witness_method) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int64
+  %122 = apply %111<@opened("C22498FA-CABF-11E5-B9A9-685B35C48C83") P>(%40) : $@convention(witness_method) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int64
+  %6 = struct $Int32 (%5 : $Builtin.Int32)
+  %8 = builtin "cmp_eq_Word"(%5 : $Builtin.Int32, %1 : $Builtin.Int32) : $Builtin.Int1
+  cond_br %8, bb3, bb2
+
+bb2:
+  %10 = integer_literal $Builtin.Int32, 1
+  %12 = integer_literal $Builtin.Int1, -1
+  %13 = builtin "sadd_with_overflow_Word"(%5 : $Builtin.Int32, %10 : $Builtin.Int32, %12 : $Builtin.Int1) : $(Builtin.Int32, Builtin.Int1)
+  %14 = tuple_extract %13 : $(Builtin.Int32, Builtin.Int1), 0
+  %15 = enum $Optional<Int32>, #Optional.some!enumelt.1, %6 : $Int32
+  %16 = unchecked_enum_data %15 : $Optional<Int32>, #Optional.some!enumelt.1
+  %17 = struct_extract %16 : $Int32, #Int32._value
+  %19 = integer_literal $Builtin.Int1, -1
+  %20 = builtin "sadd_with_overflow_Word"(%4 : $Builtin.Int32, %17 : $Builtin.Int32, %19 : $Builtin.Int1) : $(Builtin.Int32, Builtin.Int1)
+  %21 = tuple_extract %20 : $(Builtin.Int32, Builtin.Int1), 0
+  br bb1(%21 : $Builtin.Int32, %14 : $Builtin.Int32, %26: $*P, %31 : $@box Bool, %32 : $*Bool)
+
+bb3:
+  %23 = struct $Int32 (%4 : $Builtin.Int32)
+  return %23 : $Int32
+}
+
 // Don't assert on this loop. bb2 is bb1 loop's new header after rotation but
 // also bb2 loop's header.