Merge pull request #5692 from gottesmm/change_silgen_emit_qualified_loadstrong

[semantic-arc] Handle the rest of the unqualified mem opts in SILGen.

Author: gottesmm

include/swift/SIL/SILBuilder.h

@@ -459,6 +459,12 @@ class SILBuilder {
 
   LoadInst *createLoad(SILLocation Loc, SILValue LV,
                        LoadOwnershipQualifier Qualifier) {
+    assert((Qualifier != LoadOwnershipQualifier::Unqualified) ||
+           F.hasUnqualifiedOwnership() &&
+               "Unqualified inst in qualified function");
+    assert((Qualifier == LoadOwnershipQualifier::Unqualified) ||
+           F.hasQualifiedOwnership() &&
+               "Qualified inst in unqualified function");
     assert(LV->getType().isLoadable(F.getModule()));
     return insert(new (F.getModule())
                       LoadInst(getSILDebugLocation(Loc), LV, Qualifier));
@@ -481,6 +487,12 @@ class SILBuilder {
 
   StoreInst *createStore(SILLocation Loc, SILValue Src, SILValue DestAddr,
                          StoreOwnershipQualifier Qualifier) {
+    assert((Qualifier != StoreOwnershipQualifier::Unqualified) ||
+           F.hasUnqualifiedOwnership() &&
+               "Unqualified inst in qualified function");
+    assert((Qualifier == StoreOwnershipQualifier::Unqualified) ||
+           F.hasQualifiedOwnership() &&
+               "Qualified inst in unqualified function");
     return insert(new (F.getModule()) StoreInst(getSILDebugLocation(Loc), Src,
                                                 DestAddr, Qualifier));
   }

include/swift/SIL/SILValue.h

@@ -301,9 +301,11 @@ class OperandValueArrayRef {
     : Operands(operands) {}
 
   /// A simple iterator adapter.
-  class iterator {
+  class iterator : public std::iterator<std::forward_iterator_tag,
+                                        SILValue, ptrdiff_t> {
     const Operand *Ptr;
   public:
+    iterator() = default;
     iterator(const Operand *ptr) : Ptr(ptr) {}
     SILValue operator*() const { assert(Ptr); return Ptr->get(); }
     SILValue operator->() const { return operator*(); }

lib/Parse/ParseSIL.cpp

@@ -3996,16 +3996,18 @@ bool SILParser::parseSILBasicBlock(SILBuilder &B) {
 
   bool AssumeUnqualifiedOwnershipWhenParsing =
     F->getModule().getOptions().AssumeUnqualifiedOwnershipWhenParsing;
+  if (AssumeUnqualifiedOwnershipWhenParsing) {
+    F->setUnqualifiedOwnership();
+  }
   do {
     if (parseSILInstruction(BB, B))
       return true;
     // Evaluate how the just parsed instruction effects this functions Ownership
     // Qualification. For more details, see the comment on the
     // FunctionOwnershipEvaluator class.
     SILInstruction *ParsedInst = &*BB->rbegin();
-    if (AssumeUnqualifiedOwnershipWhenParsing) {
-      F->setUnqualifiedOwnership();
-    } else if (!OwnershipEvaluator.evaluate(ParsedInst)) {
+    if (!AssumeUnqualifiedOwnershipWhenParsing &&
+        !OwnershipEvaluator.evaluate(ParsedInst)) {
       P.diagnose(ParsedInst->getLoc().getSourceLoc(),
                  diag::found_unqualified_instruction_in_qualified_function,
                  F->getName());

lib/SIL/SILVerifier.cpp

@@ -1123,33 +1123,30 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     case LoadOwnershipQualifier::Unqualified:
       // We should not see loads with unqualified ownership when SILOwnership is
       // enabled.
-      requireTrueAndSILOwnership(
-          this, F.hasUnqualifiedOwnership(),
-          "Load with unqualified ownership in a qualified function");
+      require(F.hasUnqualifiedOwnership(),
+              "Load with unqualified ownership in a qualified function");
       break;
     case LoadOwnershipQualifier::Copy:
     case LoadOwnershipQualifier::Take:
-      requireTrueAndSILOwnership(
-          this, F.hasQualifiedOwnership(),
-          "Load with qualified ownership in an unqualified function");
+      require(F.hasQualifiedOwnership(),
+              "Load with qualified ownership in an unqualified function");
       // TODO: Could probably make this a bit stricter.
       require(!LI->getType().isTrivial(LI->getModule()),
               "load [copy] or load [take] can only be applied to non-trivial "
               "types");
       break;
     case LoadOwnershipQualifier::Trivial:
-      requireTrueAndSILOwnership(
-          this, F.hasQualifiedOwnership(),
-          "Load with qualified ownership in an unqualified function");
+      require(F.hasQualifiedOwnership(),
+              "Load with qualified ownership in an unqualified function");
       require(LI->getType().isTrivial(LI->getModule()),
               "A load with trivial ownership must load a trivial type");
       break;
     }
   }
 
   void checkLoadBorrowInst(LoadBorrowInst *LBI) {
-    requireTrueAndSILOwnership(
-        this, F.hasQualifiedOwnership(),
+    require(
+        F.hasQualifiedOwnership(),
         "Inst with qualified ownership in a function that is not qualified");
     require(LBI->getType().isObject(), "Result of load must be an object");
     require(LBI->getType().isLoadable(LBI->getModule()),
@@ -1161,8 +1158,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
   }
 
   void checkEndBorrowInst(EndBorrowInst *EBI) {
-    requireTrueAndSILOwnership(
-        this, F.hasQualifiedOwnership(),
+    require(
+        F.hasQualifiedOwnership(),
         "Inst with qualified ownership in a function that is not qualified");
     // We allow for end_borrow to express relationships in between addresses and
     // values, but we require that the types are the same ignoring value
@@ -1188,22 +1185,22 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     case StoreOwnershipQualifier::Unqualified:
       // We should not see loads with unqualified ownership when SILOwnership is
       // enabled.
-      requireTrueAndSILOwnership(this, F.hasUnqualifiedOwnership(),
-                                 "Invalid load with unqualified ownership");
+      require(F.hasUnqualifiedOwnership(),
+              "Qualified store in function with unqualified ownership?!");
       break;
     case StoreOwnershipQualifier::Init:
     case StoreOwnershipQualifier::Assign:
-      requireTrueAndSILOwnership(
-          this, F.hasQualifiedOwnership(),
+      require(
+          F.hasQualifiedOwnership(),
           "Inst with qualified ownership in a function that is not qualified");
       // TODO: Could probably make this a bit stricter.
       require(!SI->getSrc()->getType().isTrivial(SI->getModule()),
               "store [init] or store [assign] can only be applied to "
               "non-trivial types");
       break;
     case StoreOwnershipQualifier::Trivial:
-      requireTrueAndSILOwnership(
-          this, F.hasQualifiedOwnership(),
+      require(
+          F.hasQualifiedOwnership(),
           "Inst with qualified ownership in a function that is not qualified");
       require(SI->getSrc()->getType().isTrivial(SI->getModule()),
               "A store with trivial ownership must store a trivial type");

lib/SIL/TypeLowering.cpp

@@ -494,8 +494,7 @@ namespace {
   public:
     void emitDestroyAddress(SILBuilder &B, SILLocation loc,
                             SILValue addr) const override {
-      SILValue value =
-          B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
+      SILValue value = emitLoad(B, loc, addr, LoadOwnershipQualifier::Take);
       emitDestroyValue(B, loc, value);
     }
 
@@ -520,23 +519,29 @@ namespace {
 
     SILValue emitLoadOfCopy(SILBuilder &B, SILLocation loc, SILValue addr,
                             IsTake_t isTake) const override {
-      return B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
+      return emitLoad(B, loc, addr, LoadOwnershipQualifier::Trivial);
     }
 
     void emitStoreOfCopy(SILBuilder &B, SILLocation loc,
                          SILValue value, SILValue addr,
                          IsInitialization_t isInit) const override {
-      B.createStore(loc, value, addr, StoreOwnershipQualifier::Unqualified);
+      emitStore(B, loc, value, addr, StoreOwnershipQualifier::Trivial);
     }
 
     void emitStore(SILBuilder &B, SILLocation loc, SILValue value,
                    SILValue addr, StoreOwnershipQualifier qual) const override {
-      B.createStore(loc, value, addr, StoreOwnershipQualifier::Trivial);
+      if (B.getFunction().hasQualifiedOwnership()) {
+        B.createStore(loc, value, addr, StoreOwnershipQualifier::Trivial);
+        return;
+      }
+      B.createStore(loc, value, addr, StoreOwnershipQualifier::Unqualified);
     }
 
     SILValue emitLoad(SILBuilder &B, SILLocation loc, SILValue addr,
                       LoadOwnershipQualifier qual) const override {
-      return B.createLoad(loc, addr, LoadOwnershipQualifier::Trivial);
+      if (B.getFunction().hasQualifiedOwnership())
+        return B.createLoad(loc, addr, LoadOwnershipQualifier::Trivial);
+      return B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
     }
 
     void emitDestroyAddress(SILBuilder &B, SILLocation loc,
@@ -576,32 +581,57 @@ namespace {
 
     SILValue emitLoadOfCopy(SILBuilder &B, SILLocation loc,
                             SILValue addr, IsTake_t isTake) const override {
-      SILValue value =
-          B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
-      if (!isTake)
-        emitCopyValue(B, loc, value);
-      return value;
+      auto qual =
+          isTake ? LoadOwnershipQualifier::Take : LoadOwnershipQualifier::Copy;
+      return emitLoad(B, loc, addr, qual);
     }
 
     void emitStoreOfCopy(SILBuilder &B, SILLocation loc,
                          SILValue newValue, SILValue addr,
                          IsInitialization_t isInit) const override {
-      SILValue oldValue;
-      if (!isInit)
-        oldValue = B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
-      B.createStore(loc, newValue, addr, StoreOwnershipQualifier::Unqualified);
-      if (!isInit)
-        emitDestroyValue(B, loc, oldValue);
+      auto qual = isInit ? StoreOwnershipQualifier::Init
+                         : StoreOwnershipQualifier::Assign;
+      emitStore(B, loc, newValue, addr, qual);
     }
 
     void emitStore(SILBuilder &B, SILLocation loc, SILValue value,
                    SILValue addr, StoreOwnershipQualifier qual) const override {
-      B.createStore(loc, value, addr, qual);
+      if (B.getFunction().hasQualifiedOwnership()) {
+        B.createStore(loc, value, addr, qual);
+        return;
+      }
+
+      if (qual != StoreOwnershipQualifier::Assign) {
+        B.createStore(loc, value, addr, StoreOwnershipQualifier::Unqualified);
+        return;
+      }
+
+      // If the ownership qualifier is [assign], then we need to eliminate the
+      // old value.
+      //
+      // 1. Load old value.
+      // 2. Store new value.
+      // 3. Release old value.
+      SILValue old =
+          B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
+      B.createStore(loc, value, addr, StoreOwnershipQualifier::Unqualified);
+      B.emitDestroyValueOperation(loc, old);
     }
 
     SILValue emitLoad(SILBuilder &B, SILLocation loc, SILValue addr,
                       LoadOwnershipQualifier qual) const override {
-      return B.createLoad(loc, addr, qual);
+      if (B.getFunction().hasQualifiedOwnership())
+        return B.createLoad(loc, addr, qual);
+
+      SILValue loadValue =
+          B.createLoad(loc, addr, LoadOwnershipQualifier::Unqualified);
+
+      // If we do not have a copy, just return the value...
+      if (qual != LoadOwnershipQualifier::Copy)
+        return loadValue;
+
+      // Otherwise, emit the copy value operation.
+      return B.emitCopyValueOperation(loc, loadValue);
     }
   };
 

lib/SILGen/RValue.cpp

@@ -105,7 +105,7 @@ class ExplodeTupleValue
         // loaded.  Except it's not really an invariant, because
         // argument emission likes to lie sometimes.
         if (eltTI.isLoadable()) {
-          elt = gen.B.createLoad(loc, elt, LoadOwnershipQualifier::Unqualified);
+          elt = eltTI.emitLoad(gen.B, loc, elt, LoadOwnershipQualifier::Take);
         }
       }
 

lib/SILGen/SILGenApply.cpp

@@ -1999,8 +1999,8 @@ namespace {
 
         // If the value isn't address-only, go ahead and load.
         if (!substTL.isAddressOnly()) {
-          auto load = gen.B.createLoad(loc, value.forward(gen),
-                                       LoadOwnershipQualifier::Unqualified);
+          auto load = substTL.emitLoad(gen.B, loc, value.forward(gen),
+                                       LoadOwnershipQualifier::Take);
           value = gen.emitManagedRValueWithCleanup(load);
         }
 
@@ -3908,14 +3908,14 @@ ManagedValue SILGenFunction::emitInjectEnum(SILLocation loc,
   if (payloadMV) {
     // If the payload was indirect, we already evaluated it and
     // have a single value. Store it into the result.
-    B.createStore(loc, payloadMV.forward(*this), resultData,
-                  StoreOwnershipQualifier::Unqualified);
+    B.emitStoreValueOperation(loc, payloadMV.forward(*this), resultData,
+                              StoreOwnershipQualifier::Init);
   } else if (payloadTL.isLoadable()) {
     // The payload of this specific enum case might be loadable
     // even if the overall enum is address-only.
     payloadMV = std::move(payload).getAsSingleValue(*this, origFormalType);
-    B.createStore(loc, payloadMV.forward(*this), resultData,
-                  StoreOwnershipQualifier::Unqualified);
+    B.emitStoreValueOperation(loc, payloadMV.forward(*this), resultData,
+                              StoreOwnershipQualifier::Init);
   } else {
     // The payload is address-only. Evaluate it directly into
     // the enum.
@@ -5573,7 +5573,7 @@ RValue SILGenFunction::emitDynamicMemberRefExpr(DynamicMemberRefExpr *e,
   // Package up the result.
   auto optResult = optTemp;
   if (optTL.isLoadable())
-    optResult = B.createLoad(e, optResult, LoadOwnershipQualifier::Unqualified);
+    optResult = optTL.emitLoad(B, e, optResult, LoadOwnershipQualifier::Take);
   return RValue(*this, e, emitManagedRValueWithCleanup(optResult, optTL));
 }
 
@@ -5666,6 +5666,6 @@ RValue SILGenFunction::emitDynamicSubscriptExpr(DynamicSubscriptExpr *e,
   // Package up the result.
   auto optResult = optTemp;
   if (optTL.isLoadable())
-    optResult = B.createLoad(e, optResult, LoadOwnershipQualifier::Unqualified);
+    optResult = optTL.emitLoad(B, e, optResult, LoadOwnershipQualifier::Take);
   return RValue(*this, e, emitManagedRValueWithCleanup(optResult, optTL));
 }

lib/SILGen/SILGenBuiltin.cpp

@@ -607,8 +607,7 @@ emitBuiltinCastReference(SILGenFunction &gen,
     return gen.emitManagedBufferWithCleanup(toAddr);
 
   // Load the destination value.
-  auto result =
-      gen.B.createLoad(loc, toAddr, LoadOwnershipQualifier::Unqualified);
+  auto result = toTL.emitLoad(gen.B, loc, toAddr, LoadOwnershipQualifier::Take);
   return gen.emitManagedRValueWithCleanup(result);
 }
 
@@ -643,9 +642,7 @@ static ManagedValue emitBuiltinReinterpretCast(SILGenFunction &gen,
     // Load and retain the destination value if it's loadable. Leave the cleanup
     // on the original value since we don't know anything about it's type.
     if (toTL.isLoadable()) {
-      SILValue val =
-          gen.B.createLoad(loc, toAddr, LoadOwnershipQualifier::Unqualified);
-      return gen.emitManagedRetain(loc, val, toTL);
+      return gen.emitManagedLoadCopy(loc, toAddr, toTL);
     }
     // Leave the cleanup on the original value.
     if (toTL.isTrivial())

lib/SILGen/SILGenConstructor.cpp

@@ -275,11 +275,8 @@ void SILGenFunction::emitValueConstructor(ConstructorDecl *ctor) {
 
     if (!lowering.isAddressOnly()) {
       // Otherwise, load and return the final 'self' value.
-      selfValue =
-          B.createLoad(cleanupLoc, selfLV, LoadOwnershipQualifier::Unqualified);
-
-      // Emit a retain of the loaded value, since we return it +1.
-      selfValue = lowering.emitCopyValue(B, cleanupLoc, selfValue);
+      selfValue = lowering.emitLoad(B, cleanupLoc, selfLV,
+                                    LoadOwnershipQualifier::Copy);
 
       // Inject the self value into an optional if the constructor is failable.
       if (ctor->getFailability() != OTK_None) {
@@ -582,8 +579,9 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
     if (NeedsBoxForSelf) {
       SILLocation prologueLoc = RegularLocation(ctor);
       prologueLoc.markAsPrologue();
-      B.createStore(prologueLoc, selfArg, VarLocs[selfDecl].value,
-                    StoreOwnershipQualifier::Unqualified);
+      // SEMANTIC ARC TODO: When the verifier is complete, review this.
+      B.emitStoreValueOperation(prologueLoc, selfArg, VarLocs[selfDecl].value,
+                                StoreOwnershipQualifier::Init);
     } else {
       selfArg = B.createMarkUninitialized(selfDecl, selfArg, MUKind);
       VarLocs[selfDecl] = VarLoc::get(selfArg);
@@ -670,12 +668,19 @@ void SILGenFunction::emitClassConstructorInitializer(ConstructorDecl *ctor) {
       if (Expr *SI = ctor->getSuperInitCall())
         emitRValue(SI);
 
-      selfArg = B.createLoad(cleanupLoc, VarLocs[selfDecl].value,
-                             LoadOwnershipQualifier::Unqualified);
+      selfArg = B.emitLoadValueOperation(cleanupLoc, VarLocs[selfDecl].value,
+                                         LoadOwnershipQualifier::Copy);
+    } else {
+      // We have to do a retain because we are returning the pointer +1.
+      //
+      // SEMANTIC ARC TODO: When the verifier is complete, we will need to
+      // change this to selfArg = B.emitCopyValueOperation(...). Currently due
+      // to the way that SILGen performs folding of copy_value, destroy_value,
+      // the returned selfArg may be deleted causing us to have a
+      // dead-pointer. Instead just use the old self value since we have a
+      // class.
+      selfArg = B.emitCopyValueOperation(cleanupLoc, selfArg);
     }
-    
-    // We have to do a retain because we are returning the pointer +1.
-    selfArg = B.emitCopyValueOperation(cleanupLoc, selfArg);
 
     // Inject the self value into an optional if the constructor is failable.
     if (ctor->getFailability() != OTK_None) {

lib/SILGen/SILGenConvert.cpp

@@ -250,7 +250,7 @@ ManagedValue SILGenFunction::emitUncheckedGetOptionalValueFrom(SILLocation loc,
 
     if (optTL.isLoadable())
       payloadVal =
-          B.createLoad(loc, payloadVal, LoadOwnershipQualifier::Unqualified);
+          optTL.emitLoad(B, loc, payloadVal, LoadOwnershipQualifier::Take);
   }
 
   // Produce a correctly managed value.