[FieldSensitivePL] Fixed use-before-def handling.

rdar://111118843

Author: nate-chandler

include/swift/SIL/FieldSensitivePrunedLiveness.h

@@ -571,19 +571,22 @@ class FieldSensitivePrunedLiveBlocks {
   }
 
   /// Update this liveness result for a single use.
-  IsLive updateForUse(SILInstruction *user, unsigned bitNo) {
+  IsLive updateForUse(SILInstruction *user, unsigned bitNo,
+                      bool isUserBeforeDef) {
     assert(isInitialized());
     auto *block = user->getParent();
-    auto liveness = getBlockLiveness(block, bitNo);
-    if (liveness != Dead)
-      return liveness;
+    if (!isUserBeforeDef) {
+      auto liveness = getBlockLiveness(block, bitNo);
+      if (liveness != Dead)
+        return liveness;
+    }
     computeScalarUseBlockLiveness(block, bitNo);
     return getBlockLiveness(block, bitNo);
   }
 
   /// Update this range of liveness results for a single use.
   void updateForUse(SILInstruction *user, unsigned startBitNo,
-                    unsigned endBitNo,
+                    unsigned endBitNo, SmallBitVector const &useBeforeDefBits,
                     SmallVectorImpl<IsLive> &resultingLiveness);
 
   IsLive getBlockLiveness(SILBasicBlock *bb, unsigned bitNo) const {
@@ -854,10 +857,12 @@ class FieldSensitivePrunedLiveness {
   /// Also for flexibility, \p affectedAddress must be a derived projection from
   /// the base that \p user is affecting.
   void updateForUse(SILInstruction *user, TypeTreeLeafTypeRange span,
-                    bool lifetimeEnding);
+                    bool lifetimeEnding,
+                    SmallBitVector const &useBeforeDefBits);
 
   void updateForUse(SILInstruction *user, SmallBitVector const &bits,
-                    bool lifetimeEnding);
+                    bool lifetimeEnding,
+                    SmallBitVector const &useBeforeDefBits);
 
   void getBlockLiveness(SILBasicBlock *bb, TypeTreeLeafTypeRange span,
                         SmallVectorImpl<FieldSensitivePrunedLiveBlocks::IsLive>
@@ -1160,6 +1165,12 @@ class FieldSensitiveSSAPrunedLiveRange
     return def.first->getParentBlock() == block && def.second->contains(bit);
   }
 
+  template <typename Iterable>
+  void isUserBeforeDef(SILInstruction *user, Iterable const &iterable,
+                       SmallBitVector &useBeforeDefBits) const {
+    assert(useBeforeDefBits.none());
+  }
+
   void
   findBoundariesInBlock(SILBasicBlock *block, unsigned bitNo, bool isLiveOut,
                         FieldSensitivePrunedLivenessBoundary &boundary) const;
@@ -1243,6 +1254,36 @@ class FieldSensitiveMultiDefPrunedLiveRange
     });
   }
 
+  bool isDefBlock(SILBasicBlock *block, SmallBitVector const &bits) const {
+    assert(isInitialized());
+    auto iter = defBlocks.find(block);
+    if (!iter)
+      return false;
+    return llvm::any_of(*iter, [&](TypeTreeLeafTypeRange storedSpan) {
+      return storedSpan.intersects(bits);
+    });
+  }
+
+  /// Return true if \p user occurs before the first def in the same basic
+  /// block. In classical liveness dataflow terms, gen/kill conditions over all
+  /// users in 'bb' are:
+  ///
+  ///   Gen(bb)  |= !isDefBlock(bb) || isUserBeforeDef(bb)
+  ///   Kill(bb) &= isDefBlock(bb) && !isUserBeforeDef(bb)
+  ///
+  /// If 'bb' has no users, it is neither a Gen nor Kill. Otherwise, Gen and
+  /// Kill are complements.
+  bool isUserBeforeDef(SILInstruction *user, unsigned element) const;
+  template <typename Iterable>
+  void isUserBeforeDef(SILInstruction *user, Iterable const &iterable,
+                       SmallBitVector &useBeforeDefBits) const {
+    for (auto bit : iterable) {
+      if (isUserBeforeDef(user, bit)) {
+        useBeforeDefBits.set(bit);
+      }
+    }
+  }
+
   bool isDef(SILInstruction *inst, unsigned bit) const {
     assert(isInitialized());
     auto iter = defs.find(cast<SILNode>(inst));

lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp

@@ -505,6 +505,7 @@ void FieldSensitivePrunedLiveBlocks::computeScalarUseBlockLiveness(
 /// Terminators are not live out of the block.
 void FieldSensitivePrunedLiveBlocks::updateForUse(
     SILInstruction *user, unsigned startBitNo, unsigned endBitNo,
+    SmallBitVector const &useBeforeDefBits,
     SmallVectorImpl<IsLive> &resultingLivenessInfo) {
   assert(isInitialized());
   resultingLivenessInfo.clear();
@@ -517,10 +518,15 @@ void FieldSensitivePrunedLiveBlocks::updateForUse(
 
   for (unsigned index : indices(resultingLivenessInfo)) {
     unsigned specificBitNo = startBitNo + index;
+    auto isUseBeforeDef = useBeforeDefBits.test(specificBitNo);
     switch (resultingLivenessInfo[index]) {
     case LiveOut:
     case LiveWithin:
-      continue;
+      if (!isUseBeforeDef) {
+        continue;
+      } else {
+        LLVM_FALLTHROUGH;
+      }
     case Dead: {
       // This use block has not yet been marked live. Mark it and its
       // predecessor blocks live.
@@ -599,21 +605,21 @@ void FieldSensitivePrunedLivenessBoundary::dump() const {
 //                        MARK: FieldSensitiveLiveness
 //===----------------------------------------------------------------------===//
 
-void FieldSensitivePrunedLiveness::updateForUse(SILInstruction *user,
-                                                TypeTreeLeafTypeRange range,
-                                                bool lifetimeEnding) {
+void FieldSensitivePrunedLiveness::updateForUse(
+    SILInstruction *user, TypeTreeLeafTypeRange range, bool lifetimeEnding,
+    SmallBitVector const &useBeforeDefBits) {
   SmallVector<FieldSensitivePrunedLiveBlocks::IsLive, 8> resultingLiveness;
   liveBlocks.updateForUse(user, range.startEltOffset, range.endEltOffset,
-                          resultingLiveness);
+                          useBeforeDefBits, resultingLiveness);
 
   addInterestingUser(user, range, lifetimeEnding);
 }
 
-void FieldSensitivePrunedLiveness::updateForUse(SILInstruction *user,
-                                                SmallBitVector const &bits,
-                                                bool lifetimeEnding) {
+void FieldSensitivePrunedLiveness::updateForUse(
+    SILInstruction *user, SmallBitVector const &bits, bool lifetimeEnding,
+    SmallBitVector const &useBeforeDefBits) {
   for (auto bit : bits.set_bits()) {
-    liveBlocks.updateForUse(user, bit);
+    liveBlocks.updateForUse(user, bit, useBeforeDefBits.test(bit));
   }
 
   addInterestingUser(user, bits, lifetimeEnding);
@@ -798,74 +804,46 @@ void FieldSensitivePrunedLiveRange<LivenessWithDefs>::computeBoundary(
   }
 }
 
+bool FieldSensitiveMultiDefPrunedLiveRange::isUserBeforeDef(
+    SILInstruction *user, unsigned element) const {
+  auto *block = user->getParent();
+  if (!isDefBlock(block, element))
+    return false;
+
+  if (llvm::any_of(block->getArguments(), [this, element](SILArgument *arg) {
+        return isDef(arg, element);
+      })) {
+    return false;
+  }
+
+  auto *current = user;
+  while (true) {
+    // If user is also a def, then the use is considered before the def.
+    current = current->getPreviousInstruction();
+    if (!current)
+      return true;
+
+    if (isDef(current, element))
+      return false;
+  }
+}
+
 template <typename LivenessWithDefs>
 void FieldSensitivePrunedLiveRange<LivenessWithDefs>::updateForUse(
     SILInstruction *user, TypeTreeLeafTypeRange range, bool lifetimeEnding) {
-  PRUNED_LIVENESS_LOG(
-      llvm::dbgs()
-      << "Begin FieldSensitivePrunedLiveRange<LivenessWithDefs>::updateForUse "
-         "for: "
-      << *user);
-  PRUNED_LIVENESS_LOG(
-      llvm::dbgs() << "Looking for def instruction earlier in the block!\n");
-
-  auto *parentBlock = user->getParent();
-  for (auto ii = std::next(user->getReverseIterator()),
-            ie = parentBlock->rend();
-       ii != ie; ++ii) {
-    // If we find the def, just mark this instruction as being an interesting
-    // instruction.
-    if (asImpl().isDef(&*ii, range)) {
-      PRUNED_LIVENESS_LOG(llvm::dbgs() << "    Found def: " << *ii);
-      PRUNED_LIVENESS_LOG(llvm::dbgs()
-                 << "    Marking inst as interesting user and returning!\n");
-      addInterestingUser(user, range, lifetimeEnding);
-      return;
-    }
-  }
-
-  // Otherwise, just delegate to our parent class's update for use. This will
-  // update liveness for our predecessor blocks and add this instruction as an
-  // interesting user.
-  PRUNED_LIVENESS_LOG(llvm::dbgs() << "No defs found! Delegating to "
-                             "FieldSensitivePrunedLiveness::updateForUse.\n");
-  FieldSensitivePrunedLiveness::updateForUse(user, range, lifetimeEnding);
+  SmallBitVector useBeforeDefBits(getNumSubElements());
+  asImpl().isUserBeforeDef(user, range.getRange(), useBeforeDefBits);
+  FieldSensitivePrunedLiveness::updateForUse(user, range, lifetimeEnding,
+                                             useBeforeDefBits);
 }
 
 template <typename LivenessWithDefs>
 void FieldSensitivePrunedLiveRange<LivenessWithDefs>::updateForUse(
     SILInstruction *user, SmallBitVector const &bits, bool lifetimeEnding) {
-  PRUNED_LIVENESS_LOG(
-      llvm::dbgs()
-      << "Begin FieldSensitivePrunedLiveRange<LivenessWithDefs>::updateForUse "
-         "for: "
-      << *user);
-  PRUNED_LIVENESS_LOG(llvm::dbgs()
-                      << "Looking for def instruction earlier in the block!\n");
-
-  auto *parentBlock = user->getParent();
-  for (auto ii = std::next(user->getReverseIterator()),
-            ie = parentBlock->rend();
-       ii != ie; ++ii) {
-    // If we find the def, just mark this instruction as being an interesting
-    // instruction.
-    if (asImpl().isDef(&*ii, bits)) {
-      PRUNED_LIVENESS_LOG(llvm::dbgs() << "    Found def: " << *ii);
-      PRUNED_LIVENESS_LOG(
-          llvm::dbgs()
-          << "    Marking inst as interesting user and returning!\n");
-      addInterestingUser(user, bits, lifetimeEnding);
-      return;
-    }
-  }
-
-  // Otherwise, just delegate to our parent class's update for use. This will
-  // update liveness for our predecessor blocks and add this instruction as an
-  // interesting user.
-  PRUNED_LIVENESS_LOG(llvm::dbgs()
-                      << "No defs found! Delegating to "
-                         "FieldSensitivePrunedLiveness::updateForUse.\n");
-  FieldSensitivePrunedLiveness::updateForUse(user, bits, lifetimeEnding);
+  SmallBitVector useBeforeDefBits(getNumSubElements());
+  asImpl().isUserBeforeDef(user, bits.set_bits(), useBeforeDefBits);
+  FieldSensitivePrunedLiveness::updateForUse(user, bits, lifetimeEnding,
+                                             useBeforeDefBits);
 }
 
 //===----------------------------------------------------------------------===//

test/Interpreter/moveonly_loop.swift

@@ -0,0 +1,32 @@
+// RUN: %target-run-simple-swift(-Xfrontend -sil-verify-all) | %FileCheck %s
+// RUN: %target-run-simple-swift(-O -Xfrontend -sil-verify-all) | %FileCheck %s
+
+// REQUIRES: executable_test
+
+struct S: ~Copyable {
+  let s: String
+  init(_ s: String) { self.s = s }
+  deinit { print("deiniting \(s)") }
+}
+
+func use(_ s: borrowing S) {
+  print("using: \(s.s)")
+}
+
+@_silgen_name("f")
+func f(_ c: consuming S) {
+  repeat {
+    use(c)
+    c = S("2")
+  } while false
+}
+
+func doit() {
+  let s = S("1")
+  f(s)
+}
+
+//      CHECK: using: 1
+// CHECK-NEXT: deiniting 1
+// CHECK-NEXT: deiniting 2
+doit()

test/SILOptimizer/discard_checking.swift

@@ -473,14 +473,11 @@ struct Basics: ~Copyable {
   }
 
   consuming func reinitAfterDiscard3_bad(_ c: Color) throws {
-    // expected-error@-1 {{must consume 'self' before exiting method that discards self}}
-    // FIXME: ^ this error is related to rdar://110239087
-
     repeat {
       self = Basics() // expected-error {{cannot reinitialize 'self' after 'discard self'}}
       discard self // expected-note 2{{discarded self here}}
     } while false
-  }
+  } // expected-error {{must consume 'self' before exiting method that discards self}}
 
   consuming func reinitAfterDiscard3_ok(_ c: Color) throws {
     self = Basics()

test/SILOptimizer/field_sensitive_liverange_unit.sil

@@ -64,28 +64,11 @@ bb4:
 // CHECK: MultiDef lifetime analysis:
 // CHECK:   def in range [0, 1) instruction:   store %{{.*}} to [init] [[ADDR:%.*]] : $*C
 // CHECK:   def in range [0, 1) instruction:   store %0 to [init] [[ADDR]] : $*C
-
-// FIXME: rdar://111118843 : bb0 is live-out, but FieldSensitivePL incorrectly
-//        determines it to be LiveWithin because of its mishandling of
-//        uses-before-defs.
-//        
-//        The following HECK lines are the correct CHECK lines.  The subsequent
-//        CHECK lines capture the current erroneous behavior.
-//  HECK: bb0: LiveOut
-//  HECK: bb1: LiveWithin
-//  HECK: last user:    %{{.*}} = load [copy] [[ADDR]] : $*C
-//  HECK: boundary edge: bb2
-//  HECK: dead def:   store %0 to [init] %1 : $*C
-
-// FIXME: bb0 is not LiveWithin, it is LiveOut
-// CHECK: bb0: LiveWithin,
-// CHECK: bb1: LiveWithin,
-// CHECK: last user: %{{.*}} = load [copy] [[ADDR]] : $*C 
-// FIXME: the store of the copy is not a dead-def, it is used in bb1, by the
-//        only user added.
-// CHECK: dead def: store %2 to [init] %1 : $*C
-// CHECK: dead def: store %0 to [init] %1 : $*C
-
+// CHECK: bb0: LiveOut
+// CHECK: bb1: LiveWithin
+// CHECK: last user:    %{{.*}} = load [copy] [[ADDR]] : $*C
+// CHECK: boundary edge: bb2
+// CHECK: dead def:   store %0 to [init] %1 : $*C
 // CHECK-LABEL: end running test 1 of 1 on testMultiDefUseAddressReinit
 sil [ossa] @testMultiDefUseAddressReinit : $@convention(thin) (@owned C) -> () {
 bb0(%0: @owned $C):