SIL: fix bugs in the MemBehavior cache invalidation mechanism.

include/swift/SILOptimizer/Analysis/AliasAnalysis.h

@@ -113,19 +113,12 @@ class AliasAnalysis : public SILAnalysis {
   /// The computeMemoryBehavior() method uses this map to cache queries.
   llvm::DenseMap<MemBehaviorKeyTy, MemoryBehavior> MemoryBehaviorCache;
 
-  /// The AliasAnalysis cache can't directly map a pair of ValueBase pointers
-  /// to alias results because we'd like to be able to remove deleted pointers
-  /// without having to scan the whole map. So, instead of storing pointers we
-  /// map pointers to indices and store the indices.
-  ValueEnumerator<ValueBase*> AliasValueBaseToIndex;
-
-  /// Same as AliasValueBaseToIndex, map a pointer to the indices for
-  /// MemoryBehaviorCache.
-  ///
-  /// NOTE: we do not use the same ValueEnumerator for the alias cache, 
-  /// as when either cache is cleared, we can not clear the ValueEnumerator
-  /// because doing so could give rise to collisions in the other cache.
-  ValueEnumerator<SILNode*> MemoryBehaviorNodeToIndex;
+  /// The caches can't directly map a pair of value/instruction pointers
+  /// to results because we'd like to be able to remove deleted instruction
+  /// pointers without having to scan the whole map. So, instead of storing
+  /// pointers we map pointers to indices and store the indices.
+  ValueEnumerator<ValueBase *> ValueToIndex;
+  ValueEnumerator<SILInstruction *> InstructionToIndex;
 
   AliasResult aliasAddressProjection(SILValue V1, SILValue V2,
                                      SILValue O1, SILValue O2);
@@ -138,18 +131,7 @@ class AliasAnalysis : public SILAnalysis {
   /// Returns True if memory of type \p T1 and \p T2 may alias.
   bool typesMayAlias(SILType T1, SILType T2, const SILFunction &F);
 
-  virtual void handleDeleteNotification(SILNode *node) override {
-    assert(node->isRepresentativeSILNodeInObject());
-
-    // The pointer 'node' is going away.  We can't scan the whole cache
-    // and remove all of the occurrences of the pointer. Instead we remove
-    // the pointer from the cache that translates pointers to indices.
-    auto value = dyn_cast<ValueBase>(node);
-    if (!value) return;
-
-    AliasValueBaseToIndex.invalidateValue(value);
-    MemoryBehaviorNodeToIndex.invalidateValue(node);
-  }
+  virtual void handleDeleteNotification(SILNode *node) override;
 
   virtual bool needsNotifications() override { return true; }
 
@@ -262,6 +244,8 @@ class AliasAnalysis : public SILAnalysis {
   virtual void invalidate() override {
     AliasCache.clear();
     MemoryBehaviorCache.clear();
+    InstructionToIndex.clear();
+    ValueToIndex.clear();
   }
 
   virtual void invalidate(SILFunction *,

lib/SILOptimizer/Analysis/AliasAnalysis.cpp

@@ -538,6 +538,29 @@ bool AliasAnalysis::typesMayAlias(SILType T1, SILType T2,
   return MA;
 }
 
+void AliasAnalysis::handleDeleteNotification(SILNode *node) {
+  assert(node->isRepresentativeSILNodeInObject());
+
+  // The pointer 'node' is going away.  We can't scan the whole cache
+  // and remove all of the occurrences of the pointer. Instead we remove
+  // the pointer from the index caches.
+
+  if (auto *value = dyn_cast<ValueBase>(node))
+    ValueToIndex.invalidateValue(value);
+
+  if (auto *inst = dyn_cast<SILInstruction>(node)) {
+    InstructionToIndex.invalidateValue(inst);
+
+    // When a MultipleValueInstruction is deleted, we have to invalidate all
+    // the instruction results.
+    if (auto *mvi = dyn_cast<MultipleValueInstruction>(inst)) {
+      for (unsigned idx = 0, end = mvi->getNumResults(); idx < end; ++idx) {
+        ValueToIndex.invalidateValue(mvi->getResult(idx));
+      }
+    }
+  }
+}
+
 //===----------------------------------------------------------------------===//
 //                                Entry Points
 //===----------------------------------------------------------------------===//
@@ -557,10 +580,6 @@ AliasResult AliasAnalysis::alias(SILValue V1, SILValue V2,
   // Flush the cache if the size of the cache is too large.
   if (AliasCache.size() > AliasAnalysisMaxCacheSize) {
     AliasCache.clear();
-    AliasValueBaseToIndex.clear();
-
-    // Key is no longer valid as we cleared the AliasValueBaseToIndex.
-    Key = toAliasKey(V1, V2, TBAAType1, TBAAType2);
   }
 
   // Calculate the aliasing result and store it in the cache.
@@ -785,10 +804,10 @@ SILAnalysis *swift::createAliasAnalysis(SILModule *M) {
 
 AliasKeyTy AliasAnalysis::toAliasKey(SILValue V1, SILValue V2,
                                      SILType Type1, SILType Type2) {
-  size_t idx1 = AliasValueBaseToIndex.getIndex(V1);
+  size_t idx1 = ValueToIndex.getIndex(V1);
   assert(idx1 != std::numeric_limits<size_t>::max() &&
          "~0 index reserved for empty/tombstone keys");
-  size_t idx2 = AliasValueBaseToIndex.getIndex(V2);
+  size_t idx2 = ValueToIndex.getIndex(V2);
   assert(idx2 != std::numeric_limits<size_t>::max() &&
          "~0 index reserved for empty/tombstone keys");
   void *t1 = Type1.getOpaqueValue();

lib/SILOptimizer/Analysis/MemoryBehavior.cpp

@@ -527,10 +527,6 @@ AliasAnalysis::computeMemoryBehavior(SILInstruction *Inst, SILValue V) {
   // Flush the cache if the size of the cache is too large.
   if (MemoryBehaviorCache.size() > MemoryBehaviorAnalysisMaxCacheSize) {
     MemoryBehaviorCache.clear();
-    MemoryBehaviorNodeToIndex.clear();
-
-    // Key is no longer valid as we cleared the MemoryBehaviorNodeToIndex.
-    Key = toMemoryBehaviorKey(Inst, V);
   }
 
   // Calculate the aliasing result and store it in the cache.
@@ -549,12 +545,10 @@ AliasAnalysis::computeMemoryBehaviorInner(SILInstruction *Inst, SILValue V) {
 
 MemBehaviorKeyTy AliasAnalysis::toMemoryBehaviorKey(SILInstruction *V1,
                                                     SILValue V2) {
-  size_t idx1 =
-    MemoryBehaviorNodeToIndex.getIndex(V1->getRepresentativeSILNodeInObject());
+  size_t idx1 = InstructionToIndex.getIndex(V1);
   assert(idx1 != std::numeric_limits<size_t>::max() &&
          "~0 index reserved for empty/tombstone keys");
-  size_t idx2 = MemoryBehaviorNodeToIndex.getIndex(
-      V2->getRepresentativeSILNodeInObject());
+  size_t idx2 = ValueToIndex.getIndex(V2);
   assert(idx2 != std::numeric_limits<size_t>::max() &&
          "~0 index reserved for empty/tombstone keys");
   return {idx1, idx2};

test/SILOptimizer/mem-behavior-cache-bug.sil

@@ -0,0 +1,110 @@
+// RUN: %sil-opt -temp-rvalue-opt %s -o /dev/null 
+
+// This test exposes a bug in the MemBehavior cache invalidation mechanism.
+// With this bug, the optimizer aborts with a SIL memory lifetime failure
+// (only on linux).
+
+sil_stage canonical
+
+import Builtin
+import Swift
+import SwiftShims
+
+public struct Complex<RealType> where RealType : FloatingPoint {
+  @usableFromInline
+  @_hasStorage internal var x: RealType { get set }
+  @usableFromInline
+  @_hasStorage internal var y: RealType { get set }
+  init(_ real: RealType, _ imaginary: RealType)
+}
+
+extension Complex {
+  var phase: RealType { get }
+}
+
+extension Complex {
+  public static func log(_ z: Complex<RealType>, _ b: Bool) -> Complex<RealType>
+}
+
+sil [ossa] @callee : $@convention(method) <RealType where RealType : FloatingPoint> (@in_guaranteed Complex<RealType>) -> @out RealType
+
+sil [ossa] @test : $@convention(method) <RealType where RealType : FloatingPoint> (@in_guaranteed Complex<RealType>, Bool, @thin Complex<RealType>.Type) -> @out Complex<RealType> {
+bb0(%0 : $*Complex<RealType>, %1 : $*Complex<RealType>, %2 : $Bool, %3 : $@thin Complex<RealType>.Type):
+  debug_value_addr %1 : $*Complex<RealType>, let, name "z", argno 1
+  debug_value %2 : $Bool, let, name "b", argno 2
+  debug_value %3 : $@thin Complex<RealType>.Type, let, name "self", argno 3
+  %7 = alloc_stack $RealType, let, name "θ"
+  %8 = alloc_stack $Complex<RealType>
+  copy_addr %1 to [initialization] %8 : $*Complex<RealType>
+  %10 = function_ref @callee : $@convention(method) <τ_0_0 where τ_0_0 : FloatingPoint> (@in_guaranteed Complex<τ_0_0>) -> @out τ_0_0
+  %11 = apply %10<RealType>(%7, %8) : $@convention(method) <τ_0_0 where τ_0_0 : FloatingPoint> (@in_guaranteed Complex<τ_0_0>) -> @out τ_0_0
+  destroy_addr %8 : $*Complex<RealType>
+  dealloc_stack %8 : $*Complex<RealType>
+  %14 = integer_literal $Builtin.Int1, -1
+  %15 = struct_extract %2 : $Bool, #Bool._value
+  %16 = builtin "cmp_eq_Int1"(%15 : $Builtin.Int1, %14 : $Builtin.Int1) : $Builtin.Int1
+  cond_br %16, bb1, bb2
+
+bb1:
+  %18 = metatype $@thick RealType.Type
+  %19 = alloc_stack $RealType
+  %20 = witness_method $RealType, #FloatingPoint.infinity!getter : <Self where Self : FloatingPoint> (Self.Type) -> () -> Self : $@convention(witness_method: FloatingPoint) <τ_0_0 where τ_0_0 : FloatingPoint> (@thick τ_0_0.Type) -> @out τ_0_0
+  %21 = apply %20<RealType>(%19, %18) : $@convention(witness_method: FloatingPoint) <τ_0_0 where τ_0_0 : FloatingPoint> (@thick τ_0_0.Type) -> @out τ_0_0
+  %22 = alloc_stack $RealType
+  copy_addr [take] %7 to [initialization] %22 : $*RealType
+  %24 = alloc_stack $Complex<RealType>
+  %25 = alloc_stack $RealType
+  copy_addr [take] %19 to [initialization] %25 : $*RealType
+  %27 = struct_element_addr %24 : $*Complex<RealType>, #Complex.x
+  copy_addr [take] %25 to [initialization] %27 : $*RealType
+  dealloc_stack %25 : $*RealType
+  %30 = alloc_stack $RealType
+  copy_addr [take] %22 to [initialization] %30 : $*RealType
+  %32 = struct_element_addr %24 : $*Complex<RealType>, #Complex.y
+  copy_addr [take] %30 to [initialization] %32 : $*RealType
+  dealloc_stack %30 : $*RealType
+  copy_addr %24 to [initialization] %0 : $*Complex<RealType>
+  destroy_addr %24 : $*Complex<RealType>
+  dealloc_stack %24 : $*Complex<RealType>
+  %38 = tuple ()
+  dealloc_stack %22 : $*RealType
+  dealloc_stack %19 : $*RealType
+  dealloc_stack %7 : $*RealType
+  br bb3
+
+bb2:
+  %43 = metatype $@thick RealType.Type
+  %44 = alloc_stack $RealType
+  %45 = witness_method $RealType, #FloatingPoint.infinity!getter : <Self where Self : FloatingPoint> (Self.Type) -> () -> Self : $@convention(witness_method: FloatingPoint) <τ_0_0 where τ_0_0 : FloatingPoint> (@thick τ_0_0.Type) -> @out τ_0_0
+  %46 = apply %45<RealType>(%44, %43) : $@convention(witness_method: FloatingPoint) <τ_0_0 where τ_0_0 : FloatingPoint> (@thick τ_0_0.Type) -> @out τ_0_0
+  %47 = alloc_stack $RealType
+  copy_addr [take] %7 to [initialization] %47 : $*RealType
+  %49 = alloc_stack $Complex<RealType>
+  %50 = alloc_stack $RealType
+  copy_addr [take] %44 to [initialization] %50 : $*RealType
+  %52 = struct_element_addr %49 : $*Complex<RealType>, #Complex.x
+  copy_addr [take] %50 to [initialization] %52 : $*RealType
+  dealloc_stack %50 : $*RealType
+  %55 = alloc_stack $RealType
+  copy_addr [take] %47 to [initialization] %55 : $*RealType
+  %57 = struct_element_addr %49 : $*Complex<RealType>, #Complex.y
+  copy_addr [take] %55 to [initialization] %57 : $*RealType
+  dealloc_stack %55 : $*RealType
+  copy_addr %49 to [initialization] %0 : $*Complex<RealType>
+  destroy_addr %49 : $*Complex<RealType>
+  dealloc_stack %49 : $*Complex<RealType>
+  %63 = tuple ()
+  dealloc_stack %47 : $*RealType
+  dealloc_stack %44 : $*RealType
+  dealloc_stack %7 : $*RealType
+  br bb3
+
+bb3:
+  %68 = tuple ()
+  return %68 : $()
+}
+
+sil_property #Complex.x<τ_0_0 where τ_0_0 : FloatingPoint> ()
+sil_property #Complex.y<τ_0_0 where τ_0_0 : FloatingPoint> ()
+
+