[region-isolation] Do not transfer @out parameters!

Instead treat them as actual results.

Author: gottesmm

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -1434,8 +1434,12 @@ class PartitionOpTranslator {
 
     if (auto tryApplyInst = dyn_cast<TryApplyInst>(inst)) {
       foundResults.emplace_back(tryApplyInst->getNormalBB()->getArgument(0));
-      if (tryApplyInst->getErrorBB()->getNumArguments() > 0)
+      if (tryApplyInst->getErrorBB()->getNumArguments() > 0) {
         foundResults.emplace_back(tryApplyInst->getErrorBB()->getArgument(0));
+      }
+      for (auto indirectResults : tryApplyInst->getIndirectSILResults()) {
+        foundResults.emplace_back(indirectResults);
+      }
       return;
     }
 
@@ -1800,10 +1804,10 @@ class PartitionOpTranslator {
     };
 
     if (applySite.hasSelfArgument()) {
-      handleSILOperands(applySite.getOperandsWithoutSelf());
+      handleSILOperands(applySite.getOperandsWithoutIndirectResultsOrSelf());
       handleSILSelf(&applySite.getSelfArgumentOperand());
     } else {
-      handleSILOperands(applySite->getAllOperands());
+      handleSILOperands(applySite.getOperandsWithoutIndirectResults());
     }
 
     // non-sendable results can't be returned from cross-isolation calls without

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -553,6 +553,9 @@ void UseAfterTransferDiagnosticInferrer::initForApply(const Operand *op,
   SILInstruction *i = const_cast<SILInstruction *>(op->getUser());
   auto fai = FullApplySite::isa(i);
 
+  assert(!fai.getArgumentConvention(*op).isIndirectOutParameter() &&
+         "An indirect out parameter is never transferred");
+
   Expr *foundExpr = nullptr;
 
   // If we have self, then infer it.
@@ -677,6 +680,8 @@ void UseAfterTransferDiagnosticInferrer::init(const Operand *op) {
 
   // Otherwise, see if our operand's instruction is a transferring parameter.
   if (auto fas = FullApplySite::isa(nonConstOp->getUser())) {
+    assert(!fas.getArgumentConvention(*nonConstOp).isIndirectOutParameter() &&
+           "We should never transfer an indirect out parameter");
     if (fas.getArgumentParameterInfo(*nonConstOp)
             .hasOption(SILParameterInfo::Transferring)) {
       return initForUseOfStronglyTransferredValue(op);
@@ -962,12 +967,13 @@ struct DiagnosticEvaluator final
     auto rep = info->getValueMap().getRepresentative(transferredVal);
     LLVM_DEBUG(llvm::dbgs()
                << "    Emitting Use After Transfer Error!\n"
-               << "        ID:  %%" << transferredVal << "\n"
-               << "        Rep: " << *rep
+               << "        Transferring Inst: " << *transferringOp.getUser()
+               << "        Transferring Op Value: "
+               << transferringOp.getOperand()->get()
                << "        Require Inst: " << *partitionOp.getSourceInst()
-               << "        Transferring Op Num: "
-               << transferringOp.getOperand()->getOperandNumber() << '\n'
-               << "        Transferring Inst: " << *transferringOp.getUser());
+               << "        ID:  %%" << transferredVal << "\n"
+               << "        Rep: " << *rep << "        Transferring Op Num: "
+               << transferringOp.getOperand()->getOperandNumber() << '\n');
     transferOpToRequireInstMultiMap.insert(transferringOp.getOperand(),
                                            partitionOp.getSourceInst());
   }

test/Concurrency/transfernonsendable.sil

@@ -63,6 +63,7 @@ sil @useRawPointer : $@convention(thin) (Builtin.RawPointer) -> ()
 sil @initRawPointer : $@convention(thin) () -> Builtin.RawPointer
 
 sil @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+sil @transferIndirectWithOutResult : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0
 sil @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
 sil @initIndirect : $@convention(thin) <T> () -> @out T
 sil @initIndirectTransferring : $@convention(thin) @async <T> () -> @out T
@@ -92,3 +93,55 @@ bb0(%0 : $*{ var NonSendableKlass }):
   %9999 = tuple ()
   return %9999 : $()
 }
+
+// This doesn't error since the @out parameter is not transferred when it is initialized.
+//
+// DISCUSSION: The frontend prevents us from using such a value. But we
+// shouldn't crash on such values.
+sil [ossa] @transfer_does_not_transfer_out_parameters_1 : $@convention(thin) @async () -> () {
+bb0:
+  %0 = alloc_stack $NonSendableKlass
+  %init = function_ref @initIndirect : $@convention(thin) <T> () -> @out T
+  apply %init<NonSendableKlass>(%0) : $@convention(thin) <T> () -> @out T
+
+  %1 = alloc_stack $NonSendableKlass
+  %f = function_ref @transferIndirectWithOutResult : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0
+  apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %f<NonSendableKlass>(%1, %0) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0
+
+  %useIndirect = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+  apply %useIndirect<NonSendableKlass>(%1) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+
+  destroy_addr %1 : $*NonSendableKlass
+  dealloc_stack %1 : $*NonSendableKlass
+  destroy_addr %0 : $*NonSendableKlass
+  dealloc_stack %0 : $*NonSendableKlass
+
+  %9999 = tuple ()
+  return %9999 : $()
+}
+
+sil [ossa] @transfer_does_not_transfer_out_parameters_2 : $@convention(thin) @async () -> () {
+bb0:
+  %0 = alloc_stack $NonSendableKlass
+  %init = function_ref @initIndirect : $@convention(thin) <T> () -> @out T
+  apply %init<NonSendableKlass>(%0) : $@convention(thin) <T> () -> @out T
+
+  %1 = alloc_stack $NonSendableKlass
+  %f = function_ref @transferIndirectWithOutResult : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0
+  apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %f<NonSendableKlass>(%1, %0) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0
+
+  %f2 = function_ref @transferIndirect : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+  apply [caller_isolation=nonisolated] [callee_isolation=global_actor] %f2<NonSendableKlass>(%1) : $@convention(thin) @async <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+  // expected-warning @-1 {{transferring value of non-Sendable type 'NonSendableKlass' from nonisolated context to global actor '<null>'-isolated context}}
+  %useIndirect = function_ref @useIndirect : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+  apply %useIndirect<NonSendableKlass>(%1) : $@convention(thin) <τ_0_0> (@in_guaranteed τ_0_0) -> ()
+  // expected-note @-1 {{access here could race}}
+
+  destroy_addr %1 : $*NonSendableKlass
+  dealloc_stack %1 : $*NonSendableKlass
+  destroy_addr %0 : $*NonSendableKlass
+  dealloc_stack %0 : $*NonSendableKlass
+
+  %9999 = tuple ()
+  return %9999 : $()
+}