Merge pull request #72281 from jckarter/moveonly-optional-chain-3

SIL: Optimizer fixes for noncopyable optional chains.

Author: jckarter

include/swift/SIL/SILBuilder.h

@@ -1404,6 +1404,9 @@ class SILBuilder {
     assert(!operand->getType().isTrivial(getFunction()) &&
            "Should not be passing trivial values to this api. Use instead "
            "emitCopyValueOperation");
+    assert((getModule().getStage() == SILStage::Raw
+            || !operand->getType().isMoveOnly())
+           && "should not be copying move-only values in canonical SIL");
     return insert(new (getModule())
                       CopyValueInst(getSILDebugLocation(Loc), operand));
   }

lib/SILGen/SILGenLValue.cpp

@@ -670,9 +670,9 @@ static ManagedValue enterAccessScope(SILGenFunction &SGF, SILLocation loc,
                                      SILAccessEnforcement enforcement,
                                      std::optional<ActorIsolation> actorIso,
                                      bool noNestedConflict = false) {
-  return ManagedValue::forLValue(
-      enterAccessScope(SGF, loc, base, addr.getValue(), typeData,
-                       accessKind, enforcement, actorIso, noNestedConflict));
+  auto access = enterAccessScope(SGF, loc, base, addr.getValue(), typeData,
+                           accessKind, enforcement, actorIso, noNestedConflict);
+  return ManagedValue::forLValue(access);
 }
 
 // Find the base of the formal access at `address`. If the base requires an
@@ -4415,11 +4415,9 @@ LValue SILGenLValue::visitBindOptionalExpr(BindOptionalExpr *e,
                                    SILAccessEnforcement::Static,
                                    std::nullopt,
                                    /*no nested conflict*/ true);
-      } else {
-        // Take ownership of the base.
-        optBase = SGF.emitFormalAccessManagedRValueWithCleanup(e,
-                                                           optBase.getValue());
       }
+      // Take ownership of the base.
+      optBase = SGF.emitManagedRValueWithCleanup(optBase.getValue());
     }
   }
   // Bind the value, branching to the destination address if there's no
@@ -4440,11 +4438,16 @@ LValue SILGenLValue::visitBindOptionalExpr(BindOptionalExpr *e,
   // Reset the insertion point at the end of hasValueBB so we can
   // continue to emit code there.
   SGF.B.setInsertionPoint(someBB);
-
+  
   // Project out the payload on the success branch.  We can just use a
   // naked ValueComponent here; this is effectively a separate l-value.
   ManagedValue optPayload =
     getPayloadOfOptionalValue(SGF, e, optBase, valueTypeData, accessKind);
+  // Disable the cleanup if consuming since the consumer should pull straight
+  // from the address we give them.
+  if (optBase.getType().isMoveOnly() && isConsumeAccess(baseAccessKind)) {
+    optPayload = ManagedValue::forLValue(optPayload.forward(SGF));
+  }
   LValue valueLV;
   valueLV.add<ValueComponent>(optPayload, std::nullopt, valueTypeData,
                               /*is rvalue*/optBase.getType().isObject());

lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

@@ -1471,9 +1471,15 @@ SILInstruction *SILCombiner::visitCondBranchInst(CondBranchInst *CBI) {
     if (!CBI->getTrueArgs().empty() || !CBI->getFalseArgs().empty())
       return nullptr;
     auto EnumOperandTy = SEI->getEnumOperand()->getType();
-    // Type should be loadable
-    if (!EnumOperandTy.isLoadable(*SEI->getFunction()))
+    // Type should be loadable and copyable.
+    // TODO: Generalize to work without copying address-only or noncopyable
+    // values.
+    if (!EnumOperandTy.isLoadable(*SEI->getFunction())) {
       return nullptr;
+    }
+    if (EnumOperandTy.isMoveOnly()) {
+      return nullptr;
+    }
 
     // Result of the select_enum should be a boolean.
     if (SEI->getType() != CBI->getCondition()->getType())

test/SILGen/moveonly_optional_operations_2.swift

@@ -74,7 +74,7 @@ func consumingSwitchSubject3(ncp: consuming NCPair) {
     // CHECK:   [[PAIR_MARK:%.*]] = mark_unresolved_non_copyable_value [assignable_but_not_consumable] [[PAIR_ACCESS]]
     // CHECK:   [[FIELD:%.*]] = struct_element_addr [[PAIR_MARK]]
     // CHECK:   [[FIELD_ACCESS:%.*]] = begin_access [deinit] [static] [no_nested_conflict] [[FIELD]]
-    // CHECK:   cond_br {{.*}}, [[SOME_BB:bb[0-9]+]],
+    // CHECK:   cond_br {{.*}}, [[SOME_BB:bb[0-9]+]], [[NONE_BB:bb[0-9]+]]
     // CHECK: [[SOME_BB]]:
     // CHECK:   [[PAYLOAD_ADDR:%.*]] = unchecked_take_enum_data_addr [[FIELD_ACCESS]]
     // CHECK:   [[PAYLOAD:%.*]] = load [take] [[PAYLOAD_ADDR]]
@@ -85,6 +85,9 @@ func consumingSwitchSubject3(ncp: consuming NCPair) {
     // CHECK-NEXT: end_access [[PAIR_ACCESS]]
     // CHECK-NEXT: inject_enum_addr 
     // CHECK-NEXT: br 
+    // CHECK: [[NONE_BB]]:
+    // CHECK:   destroy_addr [[FIELD_ACCESS]]
+    // CHECK-NEXT: end_access [[FIELD_ACCESS]]
 
     switch ncp.first?.c3() {
     default: