Merge pull request #34543 from gottesmm/pr-0797da971faacb826fafd0134436240a2e678ae7

[membehavior] Fix base memory behavior/releasing of Load/Store for ownership

Author: gottesmm

include/swift/SIL/SILNodes.def

@@ -592,7 +592,7 @@ ABSTRACT_VALUE_AND_INST(SingleValueInstruction, ValueBase, SILInstruction)
 
   // Accessing memory
   SINGLE_VALUE_INST(LoadInst, load,
-                    SingleValueInstruction, MayRead, DoesNotRelease)
+                    SingleValueInstruction, MayHaveSideEffects, DoesNotRelease)
   SINGLE_VALUE_INST(LoadBorrowInst, load_borrow,
                     SingleValueInstruction, MayRead, DoesNotRelease)
   SINGLE_VALUE_INST(BeginBorrowInst, begin_borrow,
@@ -852,7 +852,7 @@ NON_VALUE_INST(BeginUnpairedAccessInst, begin_unpaired_access,
 NON_VALUE_INST(EndUnpairedAccessInst, end_unpaired_access,
                SILInstruction, MayHaveSideEffects, DoesNotRelease)
 NON_VALUE_INST(StoreInst, store,
-               SILInstruction, MayWrite, DoesNotRelease)
+               SILInstruction, MayHaveSideEffects, MayRelease)
 NON_VALUE_INST(AssignInst, assign,
                SILInstruction, MayWrite, DoesNotRelease)
 NON_VALUE_INST(AssignByWrapperInst, assign_by_wrapper,

lib/SIL/IR/SILInstruction.cpp

@@ -1029,6 +1029,36 @@ SILInstruction::MemoryBehavior SILInstruction::getMemoryBehavior() const {
                              MemoryBehavior::MayHaveSideEffects;
   }
 
+  if (auto *li = dyn_cast<LoadInst>(this)) {
+    switch (li->getOwnershipQualifier()) {
+    case LoadOwnershipQualifier::Unqualified:
+    case LoadOwnershipQualifier::Trivial:
+      return MemoryBehavior::MayRead;
+    case LoadOwnershipQualifier::Take:
+      // Take deinitializes the underlying memory. Until we separate notions of
+      // memory writing from deinitialization (since a take doesn't actually
+      // write to the memory), lets be conservative and treat it as may read
+      // write.
+      return MemoryBehavior::MayReadWrite;
+    case LoadOwnershipQualifier::Copy:
+      return MemoryBehavior::MayHaveSideEffects;
+    }
+    llvm_unreachable("Covered switch isn't covered?!");
+  }
+
+  if (auto *si = dyn_cast<StoreInst>(this)) {
+    switch (si->getOwnershipQualifier()) {
+    case StoreOwnershipQualifier::Unqualified:
+    case StoreOwnershipQualifier::Trivial:
+    case StoreOwnershipQualifier::Init:
+      return MemoryBehavior::MayWrite;
+    case StoreOwnershipQualifier::Assign:
+      // For the release.
+      return MemoryBehavior::MayHaveSideEffects;
+    }
+    llvm_unreachable("Covered switch isn't covered?!");
+  }
+
   switch (getKind()) {
 #define FULL_INST(CLASS, TEXTUALNAME, PARENT, MEMBEHAVIOR, RELEASINGBEHAVIOR)  \
   case SILInstructionKind::CLASS:                                              \
@@ -1138,6 +1168,18 @@ bool SILInstruction::mayRelease() const {
     }
     return true;
   }
+  case SILInstructionKind::StoreInst:
+    switch (cast<StoreInst>(this)->getOwnershipQualifier()) {
+    case StoreOwnershipQualifier::Unqualified:
+    case StoreOwnershipQualifier::Init:
+    case StoreOwnershipQualifier::Trivial:
+      return false;
+    case StoreOwnershipQualifier::Assign:
+      // Assign destroys the old value that was in the memory location before we
+      // write the new value into the location.
+      return true;
+    }
+    llvm_unreachable("Covered switch isn't covered?!");
   }
 }
 

lib/SILOptimizer/Analysis/MemoryBehavior.cpp

@@ -242,12 +242,15 @@ MemBehavior MemoryBehaviorVisitor::visitLoadInst(LoadInst *LI) {
   if (!mayAlias(LI->getOperand()))
     return MemBehavior::None;
 
-  // A take is modelled as a write. See MemoryBehavior::MayWrite.
-  if (LI->getOwnershipQualifier() == LoadOwnershipQualifier::Take)
-      return MemBehavior::MayReadWrite;
-
   LLVM_DEBUG(llvm::dbgs() << "  Could not prove that load inst does not alias "
-                             "pointer. Returning may read.\n");
+                             "pointer. ");
+
+  if (LI->getOwnershipQualifier() == LoadOwnershipQualifier::Take) {
+    LLVM_DEBUG(llvm::dbgs() << "Is a take so return MayReadWrite.\n");
+    return MemBehavior::MayReadWrite;
+  }
+
+  LLVM_DEBUG(llvm::dbgs() << "Not a take so returning MayRead.\n");
   return MemBehavior::MayRead;
 }
 
@@ -257,15 +260,17 @@ MemBehavior MemoryBehaviorVisitor::visitStoreInst(StoreInst *SI) {
   if (isLetValue() && (getAccessBase(SI->getDest()) != getValueAddress())) {
     return MemBehavior::None;
   }
-  // If the store dest cannot alias the pointer in question, then the
-  // specified value cannot be modified by the store.
-  if (!mayAlias(SI->getDest()))
+  // If the store dest cannot alias the pointer in question and we are not
+  // releasing anything due to an assign, then the specified value cannot be
+  // modified by the store.
+  if (!mayAlias(SI->getDest()) &&
+      SI->getOwnershipQualifier() != StoreOwnershipQualifier::Assign)
     return MemBehavior::None;
 
   // Otherwise, a store just writes.
   LLVM_DEBUG(llvm::dbgs() << "  Could not prove store does not alias inst. "
-                             "Returning MayWrite.\n");
-  return MemBehavior::MayWrite;
+                             "Returning default mem behavior.\n");
+  return SI->getMemoryBehavior();
 }
 
 MemBehavior MemoryBehaviorVisitor::visitCopyAddrInst(CopyAddrInst *CAI) {

lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp

@@ -539,12 +539,29 @@ void FunctionSideEffects::analyzeInstruction(SILInstruction *I) {
         true;
     Traps = true;
     return;
-  case SILInstructionKind::LoadInst:
-    getEffectsOn(cast<LoadInst>(I)->getOperand())->Reads = true;
+  case SILInstructionKind::LoadBorrowInst: {
+    auto *effects = getEffectsOn(cast<LoadBorrowInst>(I)->getOperand());
+    effects->Reads = true;
     return;
-  case SILInstructionKind::StoreInst:
-    getEffectsOn(cast<StoreInst>(I)->getDest())->Writes = true;
+  }
+  case SILInstructionKind::LoadInst: {
+    auto *li = cast<LoadInst>(I);
+    auto *effects = getEffectsOn(cast<LoadInst>(I)->getOperand());
+    effects->Reads = true;
+    if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Take)
+      effects->Writes = true;
+    if (li->getOwnershipQualifier() == LoadOwnershipQualifier::Copy)
+      effects->Retains = true;
+    return;
+  }
+  case SILInstructionKind::StoreInst: {
+    auto *si = cast<StoreInst>(I);
+    auto *effects = getEffectsOn(si->getDest());
+    effects->Writes = true;
+    if (si->getOwnershipQualifier() == StoreOwnershipQualifier::Assign)
+      effects->Releases = true;
     return;
+  }
   case SILInstructionKind::CondFailInst:
     Traps = true;
     return;