Merge pull request #84734 from kavon/opaque-values/fixes-2

OpaqueValues: support typed throws

include/swift/SIL/ApplySite.h

@@ -790,6 +790,25 @@ public:
     llvm_unreachable("Covered switch isn't covered?!");
   }
 
+  // For a direct error result, as a result of an @error convention, if any.
+  SILValue getDirectErrorResult() const {
+    switch (getKind()) {
+    case FullApplySiteKind::ApplyInst:
+    case FullApplySiteKind::BeginApplyInst:
+      return SILValue();
+    case FullApplySiteKind::TryApplyInst: {
+      if (getNumIndirectSILErrorResults())
+        return SILValue(); // Not a direct @error convention.
+
+      auto *errBlock = cast<TryApplyInst>(getInstruction())->getErrorBB();
+      assert(errBlock->getNumArguments() == 1 &&
+             "Expected this try_apply to have a single direct error result");
+      return errBlock->getArgument(0);
+    }
+    }
+    llvm_unreachable("Covered switch isn't covered?!");
+  }
+
   unsigned getNumIndirectSILResults() const {
     return getSubstCalleeConv().getNumIndirectSILResults();
   }

include/swift/SIL/SILFunctionConventions.h

@@ -101,6 +101,15 @@ public:
   
   SILModule &getModule() const { return *M; }
 
+  /// Is the current convention to represent address-only types in their final,
+  /// lowered form of a raw address?
+  ///
+  /// Otherwise, address-only types are instead represented opaquely as SSA
+  /// values, until the mandatory SIL pass AddressLowering has run.
+  ///
+  /// See the -enable-sil-opaque-values flag.
+  ///
+  /// \returns true iff address-only types are represented as raw addresses
   bool useLoweredAddresses() const { return loweredAddresses; }
 
   bool isTypeIndirectForIndirectParamConvention(CanType paramTy) {
@@ -261,9 +270,20 @@ public:
   }
 
   bool isArgumentIndexOfIndirectErrorResult(unsigned idx) {
-    unsigned indirectResults = getNumIndirectSILResults();
-    return idx >= indirectResults &&
-           idx < indirectResults + getNumIndirectSILErrorResults();
+    if (auto errorIdx = getArgumentIndexOfIndirectErrorResult())
+      return idx == *errorIdx;
+
+    return false;
+  }
+
+  std::optional<unsigned> getArgumentIndexOfIndirectErrorResult() {
+    unsigned hasIndirectErrorResult = getNumIndirectSILErrorResults();
+    if (!hasIndirectErrorResult)
+      return std::nullopt;
+
+    assert(hasIndirectErrorResult == 1);
+    // Error index is the first one after the indirect return results, if any.
+    return getNumIndirectSILResults();
   }
 
   unsigned getNumAutoDiffSemanticResults() const {

lib/SILGen/Cleanup.cpp

@@ -167,19 +167,17 @@ void CleanupManager::emitBranchAndCleanups(JumpDest dest,
                                            SILLocation branchLoc,
                                            ArrayRef<SILValue> args,
                                            ForUnwind_t forUnwind) {
-  emitCleanupsForBranch(dest, branchLoc, args, forUnwind);
+  emitCleanupsBeforeBranch(dest, forUnwind);
   SGF.getBuilder().createBranch(branchLoc, dest.getBlock(), args);
 }
 
-/// emitBranchAndCleanups - Emit the cleanups necessary before branching to
+/// emitCleanupsBeforeBranch - Emit the cleanups necessary before branching to
 /// the given jump destination. This does not pop the cleanup stack, nor does
 /// it emit the actual branch.
-void CleanupManager::emitCleanupsForBranch(JumpDest dest,
-                                           SILLocation branchLoc,
-                                           ArrayRef<SILValue> args,
-                                           ForUnwind_t forUnwind) {
+void CleanupManager::emitCleanupsBeforeBranch(JumpDest dest,
+                                              ForUnwind_t forUnwind) {
   SILGenBuilder &builder = SGF.getBuilder();
-  assert(builder.hasValidInsertionPoint() && "Emitting branch in invalid spot");
+  assert(builder.hasValidInsertionPoint() && "no insertion point for cleanups");
   emitCleanups(dest.getDepth(), dest.getCleanupLocation(),
                forUnwind, /*popCleanups=*/false);
 }

lib/SILGen/Cleanup.h

@@ -226,15 +226,13 @@ public:
                              ArrayRef<SILValue> args = {},
                              ForUnwind_t forUnwind = NotForUnwind);
 
-  /// Emit a branch to the given jump destination,
-  /// threading out through any cleanups we need to run. This does not pop the
-  /// cleanup stack.
+  /// Emit the cleanups necessary before branching to
+  /// the given jump destination. This does not pop the cleanup stack, nor does
+  /// it emit the actual branch.
   ///
   /// \param dest       The destination scope and block.
-  /// \param branchLoc  The location of the branch instruction.
-  /// \param args       Arguments to pass to the destination block.
-  void emitCleanupsForBranch(JumpDest dest, SILLocation branchLoc,
-                             ArrayRef<SILValue> args = {},
+  /// \param forUnwind  Whether the cleanups for this dest is for unwinding.
+  void emitCleanupsBeforeBranch(JumpDest dest,
                              ForUnwind_t forUnwind = NotForUnwind);
 
   /// emitCleanupsForReturn - Emit the top-level cleanups needed prior to a

lib/SILGen/SILGenApply.cpp

@@ -2089,10 +2089,17 @@ static void emitRawApply(SILGenFunction &SGF,
     auto result = normalBB->createPhiArgument(resultType, OwnershipKind::Owned);
     rawResults.push_back(result);
 
+    // If the error is not passed indirectly, include the expected error type
+    // according to the SILFunctionConvention.
+    std::optional<TaggedUnion<SILValue, SILType>> errorAddrOrType;
+    if (indirectErrorAddr)
+        errorAddrOrType = indirectErrorAddr;
+    else
+      errorAddrOrType = substFnConv.getSILErrorType(SGF.getTypeExpansionContext());
+
     SILBasicBlock *errorBB =
       SGF.getTryApplyErrorDest(loc, substFnType, prevExecutor,
-                               substFnType->getErrorResult(),
-                               indirectErrorAddr,
+                                *errorAddrOrType,
                                options.contains(ApplyFlags::DoesNotThrow));
 
     options -= ApplyFlags::DoesNotThrow;
@@ -5989,9 +5996,9 @@ RValue SILGenFunction::emitApply(
 
   SILValue indirectErrorAddr;
   if (substFnType->hasErrorResult()) {
-    auto errorResult = substFnType->getErrorResult();
-    if (errorResult.getConvention() == ResultConvention::Indirect) {
-      auto loweredErrorResultType = getSILType(errorResult, substFnType);
+    auto convention = silConv.getFunctionConventions(substFnType);
+    if (auto errorResult = convention.getIndirectErrorResult()) {
+      auto loweredErrorResultType = getSILType(*errorResult, substFnType);
       indirectErrorAddr = B.createAllocStack(loc, loweredErrorResultType);
       enterDeallocStackCleanup(indirectErrorAddr);
     }

lib/SILGen/SILGenFunction.h

@@ -2400,9 +2400,8 @@ public:
   SILBasicBlock *getTryApplyErrorDest(SILLocation loc,
                                       CanSILFunctionType fnTy,
                                       ExecutorBreadcrumb prevExecutor,
-                                      SILResultInfo errorResult,
-                                      SILValue indirectErrorAddr,
-                                      bool isSuppressed);
+                                      TaggedUnion<SILValue, SILType> errorAddrOrType,
+                                      bool suppressErrorPath);
 
   /// Emit a dynamic member reference.
   RValue emitDynamicMemberRef(SILLocation loc, SILValue operand,

lib/SILGen/SILGenStmt.cpp

@@ -1672,23 +1672,29 @@ void StmtEmitter::visitFailStmt(FailStmt *S) {
 
 /// Return a basic block suitable to be the destination block of a
 /// try_apply instruction.  The block is implicitly emitted and filled in.
+///
+/// \param errorAddrOrType Either the address of the indirect error result where
+/// the result will be stored, or the type of the expected Owned error value.
+///
+/// \param suppressErrorPath Should the error path be emitted as unreachable?
 SILBasicBlock *
 SILGenFunction::getTryApplyErrorDest(SILLocation loc,
                                      CanSILFunctionType fnTy,
                                      ExecutorBreadcrumb prevExecutor,
-                                     SILResultInfo errorResult,
-                                     SILValue indirectErrorAddr,
+                                     TaggedUnion<SILValue, SILType> errorAddrOrType,
                                      bool suppressErrorPath) {
   // For now, don't try to re-use destination blocks for multiple
   // failure sites.
   SILBasicBlock *destBB = createBasicBlock(FunctionSection::Postmatter);
 
   SILValue errorValue;
-  if (errorResult.getConvention() == ResultConvention::Owned) {
-    errorValue = destBB->createPhiArgument(getSILType(errorResult, fnTy),
-                                           OwnershipKind::Owned);
+  if (auto ownedErrorTy = errorAddrOrType.dyn_cast<SILType>()) {
+    errorValue = destBB->createPhiArgument(*ownedErrorTy,
+                                               OwnershipKind::Owned);
   } else {
-    errorValue = indirectErrorAddr;
+    auto errorAddr = errorAddrOrType.get<SILValue>();
+    assert(errorAddr->getType().isAddress());
+    errorValue = errorAddr;
   }
 
   assert(B.hasValidInsertionPoint() && B.insertingAtEndOfBlock());
@@ -1761,9 +1767,6 @@ void SILGenFunction::emitThrow(SILLocation loc, ManagedValue exnMV,
       } else {
         // Call the _willThrowTyped entrypoint, which handles
         // arbitrary error types.
-        SILValue tmpBuffer;
-        SILValue error;
-
         FuncDecl *entrypoint = ctx.getWillThrowTyped();
         auto genericSig = entrypoint->getGenericSignature();
         SubstitutionMap subMap = SubstitutionMap::get(
@@ -1776,18 +1779,15 @@ void SILGenFunction::emitThrow(SILLocation loc, ManagedValue exnMV,
           // Materialize the error so we can pass the address down to the
           // swift_willThrowTyped.
           exnMV = exnMV.materialize(*this, loc);
-          error = exnMV.getValue();
-          exn = exnMV.forward(*this);
-        } else {
-          // Claim the exception value.
-          exn = exnMV.forward(*this);
-          error = exn;
         }
 
         emitApplyOfLibraryIntrinsic(
             loc, entrypoint, subMap,
-            { ManagedValue::forForwardedRValue(*this, error) },
+            { exnMV },
             SGFContext());
+
+        // Claim the exception value.
+        exn = exnMV.forward(*this);
       }
     } else {
       // Claim the exception value.
@@ -1857,8 +1857,8 @@ void SILGenFunction::emitThrow(SILLocation loc, ManagedValue exnMV,
       B.createDestroyAddr(loc, exn);
   }
 
-  // Branch to the cleanup destination.
-  Cleanups.emitCleanupsForBranch(ThrowDest, loc, args, IsForUnwind);
+  // Emit clean-ups needed prior to entering throw block.
+  Cleanups.emitCleanupsBeforeBranch(ThrowDest, IsForUnwind);
   
   if (indirectErrorAddr && !exn->getType().isAddress()) {
     // Forward the error value into the return slot now. This has to happen

lib/SILOptimizer/Mandatory/AddressLowering.cpp

@@ -165,6 +165,8 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 
+#include <complex>
+
 using namespace swift;
 using llvm::SmallSetVector;
 
@@ -241,7 +243,13 @@ visitCallResults(FullApplySite apply,
   if (auto *destructure = getCallDestructure(apply)) {
     return visitCallMultiResults(destructure, fnConv, visitor);
   }
-  return visitor(apply.getResult(), *fnConv.getDirectSILResults().begin());
+  // Visit the single direct result, if any.
+  auto directResults = fnConv.getDirectSILResults();
+  if (!directResults.empty()) {
+    assert(std::distance(directResults.begin(), directResults.end()) == 1);
+    return visitor(apply.getResult(), *directResults.begin());
+  }
+  return true;
 }
 
 /// Return true if the given value is either a "fake" tuple that represents all
@@ -683,8 +691,9 @@ static void convertDirectToIndirectFunctionArgs(AddressLoweringState &pass) {
 }
 
 /// Before populating the ValueStorageMap, insert function arguments for any
-/// @out result type. Return the number of indirect result arguments added.
-static unsigned insertIndirectReturnArgs(AddressLoweringState &pass) {
+/// @out result type or @error_indirect.
+/// \returns the number of indirect result and error arguments added.
+static unsigned insertIndirectReturnOrErrorArgs(AddressLoweringState &pass) {
   auto &astCtx = pass.getModule()->getASTContext();
   auto typeCtx = pass.function->getTypeExpansionContext();
   auto *declCtx = pass.function->getDeclContext();
@@ -695,14 +704,14 @@ static unsigned insertIndirectReturnArgs(AddressLoweringState &pass) {
     declCtx = pass.function->getModule().getSwiftModule();
   }
 
-  unsigned argIdx = 0;
-  for (auto resultTy : pass.loweredFnConv.getIndirectSILResultTypes(typeCtx)) {
+  auto createIndirectResult = [&](SILType resultTy, StringRef internalName,
+                                  unsigned argIdx) {
     auto resultTyInContext = pass.function->mapTypeIntoContext(resultTy);
     auto bodyResultTy = pass.function->getModule().Types.getLoweredType(
         resultTyInContext.getASTType(), *pass.function);
-    auto var = new (astCtx) ParamDecl(
-        SourceLoc(), SourceLoc(), astCtx.getIdentifier("$return_value"),
-        SourceLoc(), astCtx.getIdentifier("$return_value"), declCtx);
+    auto var = new (astCtx)
+        ParamDecl(SourceLoc(), SourceLoc(), astCtx.getIdentifier(internalName),
+                  SourceLoc(), astCtx.getIdentifier(internalName), declCtx);
     var->setSpecifier(ParamSpecifier::InOut);
 
     SILFunctionArgument *funcArg =
@@ -713,10 +722,22 @@ static unsigned insertIndirectReturnArgs(AddressLoweringState &pass) {
     //
     // This is the only case where a value defines its own storage.
     pass.valueStorageMap.insertValue(funcArg, funcArg);
+  };
+
+  unsigned argIdx = 0;
+  for (auto resultTy : pass.loweredFnConv.getIndirectSILResultTypes(typeCtx))
+    createIndirectResult(resultTy, "$return_value", argIdx++);
 
-    ++argIdx;
-  }
   assert(argIdx == pass.loweredFnConv.getNumIndirectSILResults());
+
+  // Next, add the indirect error result, if needed.
+  // This must happen after all indirect result types have been added, to match
+  // the convention.
+  if (auto errorTy = pass.loweredFnConv.getIndirectErrorResultType(typeCtx))
+    createIndirectResult(errorTy, "$error", argIdx++);
+
+  assert(argIdx == pass.loweredFnConv.getNumIndirectSILResults() +
+                       pass.loweredFnConv.getNumIndirectSILErrorResults());
   return argIdx;
 }
 
@@ -907,8 +928,8 @@ static void prepareValueStorage(AddressLoweringState &pass) {
   // Fixup this function's argument types with temporary loads.
   convertDirectToIndirectFunctionArgs(pass);
 
-  // Create a new function argument for each indirect result.
-  insertIndirectReturnArgs(pass);
+  // Create a new function argument for each indirect result or error.
+  insertIndirectReturnOrErrorArgs(pass);
 
   // Populate valueStorageMap.
   OpaqueValueVisitor(pass).mapValueStorage();
@@ -2348,16 +2369,36 @@ protected:
     return bb->begin();
   }
 
+  SILBasicBlock::iterator getErrorInsertionPoint() {
+    switch (apply.getKind()) {
+      case FullApplySiteKind::ApplyInst:
+      case FullApplySiteKind::BeginApplyInst:
+        llvm_unreachable("no error block exists for these instructions");
+
+      case FullApplySiteKind::TryApplyInst:
+        return cast<TryApplyInst>(apply)->getErrorBB()->begin();
+    }
+  }
+
   void makeIndirectArgs(MutableArrayRef<SILValue> newCallArgs);
 
   SILBasicBlock::iterator getResultInsertionPoint();
 
-  SILValue materializeIndirectResultAddress(SILValue oldResult, SILType argTy);
+  /// Indicator for the kind of output value from apply instructions.
+  enum class ApplyOutput {
+    Result,  // A returned value
+    Error    // A thrown error
+  };
+
+  SILValue materializeIndirectOutputAddress(ApplyOutput kind,
+                                            SILValue oldResult, SILType argTy);
 
   void rewriteApply(ArrayRef<SILValue> newCallArgs);
 
   void rewriteTryApply(ArrayRef<SILValue> newCallArgs);
 
+  void replaceBlockArg(SILArgument *arg, SILType newType, SILValue repl);
+
   void replaceDirectResults(DestructureTupleInst *oldDestructure);
 };
 } // end anonymous namespace
@@ -2510,7 +2551,8 @@ void ApplyRewriter::makeIndirectArgs(MutableArrayRef<SILValue> newCallArgs) {
            "canonical call results are always direct");
 
     if (loweredCalleeConv.isSILIndirect(resultInfo)) {
-      SILValue indirectResultAddr = materializeIndirectResultAddress(
+      SILValue indirectResultAddr = materializeIndirectOutputAddress(
+          ApplyOutput::Result,
           result, loweredCalleeConv.getSILType(resultInfo, typeCtx));
       // Record the new indirect call argument.
       newCallArgs[newResultArgIdx++] = indirectResultAddr;
@@ -2519,6 +2561,18 @@ void ApplyRewriter::makeIndirectArgs(MutableArrayRef<SILValue> newCallArgs) {
   };
   visitCallResults(apply, visitCallResult);
 
+  // Handle a try_apply for @error_indirect, who in the opaque convention has
+  // a direct error result, but needs an indirect error result when lowered.
+  if (auto errResult = apply.getDirectErrorResult()) {
+    if (auto errorInfo = loweredCalleeConv.getIndirectErrorResult()) {
+      SILValue indirectErrorAddr = materializeIndirectOutputAddress(
+          ApplyOutput::Error,
+          errResult, loweredCalleeConv.getSILType(*errorInfo, typeCtx));
+      // Record the new indirect call argument.
+      newCallArgs[newResultArgIdx++] = indirectErrorAddr;
+    }
+  }
+
   // Append the existing call arguments to the SIL argument list. They were
   // already lowered to addresses by CallArgRewriter.
   assert(newResultArgIdx == loweredCalleeConv.getSILArgIndexOfFirstParam());
@@ -2544,13 +2598,16 @@ SILBasicBlock::iterator ApplyRewriter::getResultInsertionPoint() {
   }
 }
 
-/// Return the storage address for the indirect result corresponding to the
+/// Return the storage address for the indirect output corresponding to the
 /// \p oldResult. Allocate temporary argument storage for an
-/// indirect result that isn't mapped to storage because it is either loadable
+/// indirect output that isn't mapped to storage because it is either loadable
 /// or unused.
 ///
 /// \p oldResult is invalid for an unused result.
-SILValue ApplyRewriter::materializeIndirectResultAddress(SILValue oldResult,
+///
+/// \param kind is the kind of output we're materializing an address for.
+SILValue ApplyRewriter::materializeIndirectOutputAddress(ApplyOutput kind,
+                                                         SILValue oldResult,
                                                          SILType argTy) {
   if (oldResult && oldResult->getType().isAddressOnly(*pass.function)) {
     // Results that project into their uses have not yet been materialized.
@@ -2573,7 +2630,10 @@ SILValue ApplyRewriter::materializeIndirectResultAddress(SILValue oldResult,
   if (oldResult && !oldResult->use_empty()) {
     // Insert reloads immediately after the call. Get the reload insertion
     // point after emitting dealloc to ensure the reload happens first.
-    auto reloadBuilder = pass.getBuilder(getResultInsertionPoint());
+    auto insertionPt = kind == ApplyOutput::Result
+                           ? getResultInsertionPoint()
+                           : getErrorInsertionPoint();
+    auto reloadBuilder = pass.getBuilder(insertionPt);
 
     // This is a formally indirect argument, but is loadable.
     auto *loadInst = reloadBuilder.createTrivialLoadOr(
@@ -2694,6 +2754,22 @@ void ApplyRewriter::convertBeginApplyWithOpaqueYield() {
   }
 }
 
+/// Utility to replace all uses of a block's argument with a SILValue,
+/// preserving the argument by creating a fresh, unused argument of the given
+/// type.
+///
+/// \param arg the block argument to be replaced
+/// \param newType the type of the fresh block argument to be created
+/// \param repl the value to replace uses of the old argument with
+void ApplyRewriter::replaceBlockArg(SILArgument *arg, SILType newType,
+                                    SILValue repl) {
+  const unsigned idx = arg->getIndex();
+  auto ownership = newType.isTrivial(*pass.function) ? OwnershipKind::None
+                                                     : OwnershipKind::Owned;
+  arg->replaceAllUsesWith(repl);
+  arg->getParent()->replacePhiArgument(idx, newType, ownership, arg->getDecl());
+}
+
 // Replace \p tryApply with a new try_apply using \p newCallArgs.
 //
 // If the old result was a single opaque value, then create and return a
@@ -2758,7 +2834,37 @@ void ApplyRewriter::convertBeginApplyWithOpaqueYield() {
 //   // no uses of %d1
 //
 void ApplyRewriter::rewriteTryApply(ArrayRef<SILValue> newCallArgs) {
-  auto typeCtx = pass.function->getTypeExpansionContext();
+  // Util to rewrite the argument to one of the successor blocks of a try_apply.
+  auto rewriteTryApplySuccBlockArg = [&](SILArgument *arg, SILType newArgTy,
+                                         SILBuilder &builder) {
+    assert(arg);
+    assert(arg->getIndex() == 0);
+
+    // Handle a single opaque result value.
+    if (pass.valueStorageMap.contains(arg)) {
+      // Storage was materialized by materializeIndirectOutputAddress.
+      auto &origStorage = pass.valueStorageMap.getStorage(arg);
+      assert(origStorage.isRewritten);
+      (void)origStorage;
+
+      // Rewriting try_apply with a new function type requires erasing the opaque
+      // block argument.  Create a dummy load-copy until all uses have been
+      // rewritten.
+      LoadInst *loadArg = builder.createLoad(
+          callLoc, origStorage.storageAddress, LoadOwnershipQualifier::Copy);
+
+      pass.valueStorageMap.replaceValue(arg, loadArg);
+      replaceBlockArg(arg, newArgTy, loadArg);
+      return;
+    }
+    // Loadable results were loaded by materializeIndirectOutputAddress.
+    // Temporarily redirect all uses to Undef. They will be fixed in
+    // replaceDirectResults().
+    auto undefVal =
+        SILUndef::get(pass.function, arg->getType().getAddressType());
+    replaceBlockArg(arg, newArgTy, undefVal);
+  };
+
   auto *tryApply = cast<TryApplyInst>(apply.getInstruction());
 
   auto *newCallInst = argBuilder.createTryApply(
@@ -2766,45 +2872,37 @@ void ApplyRewriter::rewriteTryApply(ArrayRef<SILValue> newCallArgs) {
       tryApply->getNormalBB(), tryApply->getErrorBB(),
       tryApply->getApplyOptions(), tryApply->getSpecializationInfo());
 
-  auto *resultArg = cast<SILArgument>(apply.getResult());
-
-  auto replaceTermResult = [&](SILValue newResultVal) {
-    SILType resultTy = loweredCalleeConv.getSILResultType(typeCtx);
-    auto ownership = resultTy.isTrivial(*pass.function) ? OwnershipKind::None
-                                                        : OwnershipKind::Owned;
-
-    resultArg->replaceAllUsesWith(newResultVal);
-    assert(resultArg->getIndex() == 0);
-    resultArg->getParent()->replacePhiArgument(0, resultTy, ownership,
-                                               resultArg->getDecl());
-  };
   // Immediately delete the old try_apply (old applies hang around until
   // dead code removal because they directly define values).
   pass.deleter.forceDelete(tryApply);
   this->apply = FullApplySite(newCallInst);
 
-  // Handle a single opaque result value.
-  if (pass.valueStorageMap.contains(resultArg)) {
-    // Storage was materialized by materializeIndirectResultAddress.
-    auto &origStorage = pass.valueStorageMap.getStorage(resultArg);
-    assert(origStorage.isRewritten);
-    (void)origStorage;
+  auto typeCtx = pass.function->getTypeExpansionContext();
+  auto &astCtx = pass.getModule()->getASTContext();
+  auto calleeFnTy = apply.getSubstCalleeType();
+  SILArgument *resultArg = nullptr, *errorArg = nullptr;
 
-    // Rewriting try_apply with a new function type requires erasing the opaque
-    // block argument.  Create a dummy load-copy until all uses have been
-    // rewritten.
-    LoadInst *loadArg = resultBuilder.createLoad(
-        callLoc, origStorage.storageAddress, LoadOwnershipQualifier::Copy);
-
-    pass.valueStorageMap.replaceValue(resultArg, loadArg);
-    replaceTermResult(loadArg);
-    return;
+  if (calleeFnTy->hasIndirectFormalResults()) {
+    resultArg = cast<SILArgument>(apply.getResult());
+    SILType resultTy = loweredCalleeConv.getSILResultType(typeCtx);
+    rewriteTryApplySuccBlockArg(resultArg, resultTy, resultBuilder);
   }
-  // Loadable results were loaded by materializeIndirectResultAddress.
-  // Temporarily redirect all uses to Undef. They will be fixed in
-  // replaceDirectResults().
-  replaceTermResult(
-      SILUndef::get(pass.function, resultArg->getType().getAddressType()));
+
+  if (calleeFnTy->hasIndirectErrorResult()) {
+    errorArg = cast<SILArgument>(apply.getDirectErrorResult());
+    auto *errBB = errorArg->getParentBlock();
+    SILBuilder errorBuilder = pass.getBuilder(getErrorInsertionPoint());
+
+    // A made-up type, since we're going to delete the argument.
+    auto dummyTy = SILType::getEmptyTupleType(astCtx);
+    rewriteTryApplySuccBlockArg(errorArg, dummyTy, errorBuilder);
+
+    // We must delete the error block's argument for an @error_indirect
+    assert(errBB->getNumArguments() == 1);
+    errBB->eraseArgument(0);
+  }
+
+  assert((resultArg || errorArg) && "should have rewritten something?");
 }
 
 // Replace all formally direct results by rewriting the destructure_tuple.
@@ -3062,9 +3160,11 @@ public:
       : pass(pass), opaqueFnConv(pass.function->getConventions()) {}
 
   void rewriteReturns();
+  void rewriteThrows();
 
 protected:
   void rewriteReturn(ReturnInst *returnInst);
+  void rewriteThrow(ThrowInst *throwInst);
 
   void rewriteElement(SILValue oldResult, SILArgument *newResultArg,
                       SILBuilder &returnBuilder);
@@ -3079,18 +3179,42 @@ void ReturnRewriter::rewriteReturns() {
   }
 }
 
-void ReturnRewriter::rewriteReturn(ReturnInst *returnInst) {
-  auto &astCtx = pass.getModule()->getASTContext();
-  auto typeCtx = pass.function->getTypeExpansionContext();
+void ReturnRewriter::rewriteThrows() {
+  for (SILInstruction *termInst : pass.exitingInsts) {
+    if (auto *throwInst = dyn_cast<ThrowInst>(termInst))
+      rewriteThrow(throwInst);
+    else
+      assert(isa<ReturnInst>(termInst));
+  }
+}
 
-  // Find the point before allocated storage has been deallocated.
-  auto insertPt = SILBasicBlock::iterator(returnInst);
-  for (auto bbStart = returnInst->getParent()->begin(); insertPt != bbStart;
+// Find the point just before allocated storage has been deallocated,
+// immediately prior to this instruction.
+static SILBasicBlock::iterator beforeStorageDeallocs(SILInstruction *inst) {
+  auto insertPt = SILBasicBlock::iterator(inst);
+  for (auto bbStart = inst->getParent()->begin(); insertPt != bbStart;
        --insertPt) {
     if (!isa<DeallocStackInst>(*std::prev(insertPt)))
       break;
   }
-  auto returnBuilder = pass.getBuilder(insertPt);
+  return insertPt;
+}
+
+void ReturnRewriter::rewriteThrow(ThrowInst *throwInst) {
+  auto idx = pass.loweredFnConv.getArgumentIndexOfIndirectErrorResult();
+  SILArgument *errorResultAddr = pass.function->getArgument(idx.value());
+
+  auto throwBuilder = pass.getBuilder(beforeStorageDeallocs(throwInst));
+  rewriteElement(throwInst->getOperand(), errorResultAddr, throwBuilder);
+  throwBuilder.createThrowAddr(throwInst->getLoc());
+  pass.deleter.forceDelete(throwInst);
+}
+
+void ReturnRewriter::rewriteReturn(ReturnInst *returnInst) {
+  auto &astCtx = pass.getModule()->getASTContext();
+  auto typeCtx = pass.function->getTypeExpansionContext();
+
+  auto returnBuilder = pass.getBuilder(beforeStorageDeallocs(returnInst));
 
   // Gather direct function results.
   unsigned numOldResults = opaqueFnConv.getNumDirectSILResults();
@@ -3126,7 +3250,7 @@ void ReturnRewriter::rewriteReturn(ReturnInst *returnInst) {
 
   assert(newDirectResults.size() ==
          pass.loweredFnConv.getNumDirectSILResults());
-  assert(newResultArgIdx == pass.loweredFnConv.getSILArgIndexOfFirstParam());
+  assert(newResultArgIdx == pass.loweredFnConv.getNumIndirectSILResults());
 
   // Generate a new return_inst for the new direct results.
   SILValue newReturnVal;
@@ -3532,6 +3656,11 @@ protected:
     // opaque value rewriting.
   }
 
+  void visitThrowInst(ThrowInst *throwInst) {
+    // Throws are rewritten for any function with an @error_indirect after
+    // opaque value rewriting.
+  }
+
   void visitStoreBorrowInst(StoreBorrowInst *sbi) {
     auto addr = addrMat.materializeAddress(use->get());
     SmallVector<Operand *, 4> uses(sbi->getUses());
@@ -4234,7 +4363,9 @@ static void rewriteIndirectApply(ApplySite anyApply,
   switch (apply.getKind()) {
   case FullApplySiteKind::ApplyInst:
   case FullApplySiteKind::TryApplyInst: {
-    if (!apply.getSubstCalleeType()->hasIndirectFormalResults()) {
+    auto calleeFnTy = apply.getSubstCalleeType();
+    if (!calleeFnTy->hasIndirectFormalResults() &&
+        !calleeFnTy->hasIndirectErrorResult()) {
       return;
     }
     // If the call has indirect results and wasn't already rewritten, rewrite it
@@ -4342,6 +4473,8 @@ static void rewriteFunction(AddressLoweringState &pass) {
     ReturnRewriter(pass).rewriteReturns();
   if (pass.function->getLoweredFunctionType()->hasIndirectFormalYields())
     YieldRewriter(pass).rewriteYields();
+  if (pass.function->getLoweredFunctionType()->hasIndirectErrorResult())
+    ReturnRewriter(pass).rewriteThrows();
 }
 
 // Given an array of terminator operand values, produce an array of

test/SILGen/opaque_values_silgen.swift

@@ -875,7 +875,7 @@ struct Twople<T> {
   }
 }
 
-// CHECK-LABEL: sil{{.*}} [ossa] @throwTypedValue : {{.*}} {
+// CHECK-LABEL: sil{{.*}} [ossa] @throwTypedValue : {{.*}} -> @error Err {
 // CHECK:       bb0([[E:%[^,]+]] :
 // CHECK:         [[SWIFT_WILL_THROW_TYPED:%[^,]+]] = function_ref @swift_willThrowTyped
 // CHECK:         apply [[SWIFT_WILL_THROW_TYPED]]<Err>([[E]])
@@ -884,6 +884,57 @@ struct Twople<T> {
 @_silgen_name("throwTypedValue")
 func throwTypedValue(_ e: Err) throws(Err) { throw e }
 
+// CHECK-LABEL: sil{{.*}} [ossa] @callTypedThrowsFunc : {{.*}} -> @error any Error {
+// CHECK:       bb0:
+// CHECK:         [[TYPED_THROW_FN:%[^,]+]] = function_ref @throwTypedValue
+// CHECK:         try_apply [[TYPED_THROW_FN]]({{%[0-9]+}}) : $@convention(thin) (Err) -> @error Err, normal bb1, error bb2
+//
+// CHECK:       bb1({{%[0-9]+}} : $()):
+// CHECK:         return
+//
+// CHECK:       bb2([[E:%[^,]+]] : $Err):
+// CHECK:         [[STACK_ALLOC:%[^,]+]] = alloc_stack $any Error
+// CHECK:         [[BOX:%[^,]+]] = project_existential_box $Err
+// CHECK:         store [[E]] to [trivial] [[BOX]]
+// CHECK:         [[ANY_ERROR:%[^,]+]] = load [take] [[STACK_ALLOC]]
+// CHECK:         throw [[ANY_ERROR]]
+// CHECK-LABEL: } // end sil function 'callTypedThrowsFunc'
+@_silgen_name("callTypedThrowsFunc")
+func callTypedThrowsFunc() throws {
+  try throwTypedValue(Err())
+}
+
+// CHECK-LABEL: sil{{.*}} [ossa] @throwTypedValueGeneric : {{.*}} <GenErr where GenErr : Error> (@in_guaranteed GenErr) -> @error_indirect GenErr {
+// CHECK:       bb0([[BORROWED_ERR:%[^,]+]] : @guaranteed $GenErr):
+// CHECK:         [[E:%[^,]+]] = copy_value [[BORROWED_ERR]]
+// CHECK:         [[SWIFT_WILL_THROW_TYPED:%[^,]+]] = function_ref @swift_willThrowTyped
+// CHECK:         apply [[SWIFT_WILL_THROW_TYPED]]<GenErr>([[E]])
+// CHECK:         throw [[E]]
+// CHECK-LABEL: } // end sil function 'throwTypedValueGeneric'
+@_silgen_name("throwTypedValueGeneric")
+func throwTypedValueGeneric<GenErr: Error>(_ e: GenErr) throws(GenErr) { throw e }
+
+// CHECK-LABEL: sil{{.*}} [ossa] @callTypedThrowsFuncGeneric : {{.*}} -> @error any Error {
+// CHECK:       bb0:
+// CHECK:         [[ERR_VAL:%[^,]+]] = apply {{.*}} -> Err
+// CHECK:         [[GEN_TYPED_THROW_FN:%[^,]+]] = function_ref @throwTypedValueGeneric
+// CHECK:         try_apply [[GEN_TYPED_THROW_FN]]<Err>([[ERR_VAL]]) : $@convention(thin) <τ_0_0 where τ_0_0 : Error> (@in_guaranteed τ_0_0) -> @error_indirect τ_0_0, normal bb1, error bb2
+//
+// CHECK:       bb1({{%[0-9]+}} : $()):
+// CHECK:         return
+//
+// CHECK:       bb2([[E:%[^,]+]] : $Err):
+// CHECK:         [[STACK_ALLOC:%[^,]+]] = alloc_stack $any Error
+// CHECK:         [[BOX:%[^,]+]] = project_existential_box $Err
+// CHECK:         store [[E]] to [trivial] [[BOX]]
+// CHECK:         [[ANY_ERROR:%[^,]+]] = load [take] [[STACK_ALLOC]]
+// CHECK:         throw [[ANY_ERROR]]
+// CHECK-LABEL: } // end sil function 'callTypedThrowsFuncGeneric'
+@_silgen_name("callTypedThrowsFuncGeneric")
+func callTypedThrowsFuncGeneric() throws {
+  try throwTypedValueGeneric(Err())
+}
+
 struct Err : Error {}
 
 // CHECK-LABEL: sil{{.*}} [ossa] @copy_expr_generic : {{.*}} {

test/SILOptimizer/address_lowering.sil

@@ -29,6 +29,8 @@ enum Optional<T> {
 
 protocol Error {}
 
+struct Bad: Error {}
+
 struct I {}
 
 class Klass {}
@@ -109,6 +111,8 @@ sil [ossa] @takeTuple : $@convention(thin) <τ_0_0> (@in_guaranteed (τ_0_0, C))
 
 sil [ossa] @eraseToAny : $@convention(thin) <T> (@in_guaranteed T) -> @out Any
 sil [ossa] @produceInt : $@convention(thin) () -> Int
+sil [ossa] @produceBool : $@convention(thin) () -> Bool
+sil [ossa] @produceBad : $@convention(thin) () -> Bad
 
 sil [ossa] @takeIn : $@convention(thin) <T> (@in T) -> ()
 sil [ossa] @takeInGuaranteed : $@convention(thin) <T> (@in_guaranteed T) -> ()
@@ -1454,6 +1458,142 @@ bad(%41 : @owned $any Error):
   throw %41 : $any Error
 }
 
+// While an '@error Error' turns into an '@error any Error', an '@error' of a concrete type remains as-is.
+// --------
+// CHECK-LABEL: sil [ossa] @f270_typedThrows_throwConcrete : $@convention(thin) (Bad) -> (Bad, @error Bad) {
+// CHECK:       bb0(%0 : $Bad):
+// CHECK:         return %0 : $Bad
+// CHECK:         throw %0 : $Bad
+// CHECK-LABEL: } // end sil function 'f270_typedThrows_throwConcrete'
+sil [ossa] @f270_typedThrows_throwConcrete : $@convention(thin) (Bad) -> (Bad, @error Bad) {
+bb0(%0 : $Bad):
+  %fn = function_ref @produceBool : $@convention(thin) () -> Bool
+  %cond = apply %fn() : $@convention(thin) () -> Bool
+  cond_br %cond, doThrow, doReturn
+
+doThrow:
+  throw %0
+
+doReturn:
+  return %0
+}
+
+// CHECK-LABEL: sil [ossa] @f271_typedThrows_callConcrete : $@convention(thin) () -> () {
+// CHECK:         try_apply {{.*}} : $@convention(thin) (Bad) -> (Bad, @error Bad), normal bb2, error bb1
+// CHECK:         bb1({{.*}} : $Bad)
+// CHECK:         bb2({{.*}} : $Bad)
+// CHECK-LABEL: } // end sil function 'f271_typedThrows_callConcrete'
+sil [ossa] @f271_typedThrows_callConcrete : $@convention(thin) () -> () {
+bb0:
+  %fn = function_ref @produceBad : $@convention(thin) () -> Bad
+  %arg = apply %fn() : $@convention(thin) () -> Bad
+  %callee = function_ref @f270_typedThrows_throwConcrete : $@convention(thin) (Bad) -> (Bad, @error Bad)
+  try_apply %callee(%arg) : $@convention(thin) (Bad) -> (Bad, @error Bad), normal normalBB, error errorBB
+
+normalBB(%r : $Bad):
+  ignored_use %r
+  br exitBB
+
+errorBB(%e : $Bad):
+  ignored_use %e
+  br exitBB
+
+exitBB:
+  %t = tuple ()
+  return %t
+}
+
+// Handle throwing a generic type conforming to Error via @error_indirect
+// --------
+// CHECK-LABEL: sil [ossa] @f272_typedThrows_throwGeneric : $@convention(thin) <GenBad, Result where GenBad : Error> (@in_guaranteed GenBad, @in_guaranteed Result) -> (@out Result, @error_indirect GenBad) {
+// CHECK:       bb0(%0 : $*Result, %1 : $*GenBad, %2 : $*GenBad, %3 : $*Result):
+// CHECK:         cond_br {{.*}}, bb2, bb1
+//
+// CHECK:       bb1:
+// CHECK:         copy_addr %3 to [init] %0 : $*Result
+// CHECK:         return {{.*}} : $()
+//
+// CHECK:       bb2:
+// CHECK:         [[STACK:%[^,]+]] = alloc_stack $GenBad
+// CHECK:         copy_addr %2 to [init] [[STACK]] : $*GenBad
+// CHECK:         copy_addr [take] [[STACK]] to [init] %1 : $*GenBad
+// CHECK:         dealloc_stack [[STACK]] : $*GenBad
+// CHECK:         throw_addr
+// CHECK-LABEL: } // end sil function 'f272_typedThrows_throwGeneric'
+sil [ossa] @f272_typedThrows_throwGeneric : $@convention(thin) <GenBad, Result where GenBad : Error> (@in_guaranteed GenBad, @in_guaranteed Result) -> (@out Result, @error_indirect GenBad) {
+bb0(%e : @guaranteed $GenBad, %r : @guaranteed $Result):
+  %fn = function_ref @produceBool : $@convention(thin) () -> Bool
+  %cond = apply %fn() : $@convention(thin) () -> Bool
+  cond_br %cond, doThrow, doReturn
+
+doReturn:
+  %rCopy = copy_value %r
+  return %rCopy
+
+doThrow:
+  %eCopy = copy_value %e
+  throw %eCopy
+}
+
+// CHECK-LABEL:  sil [ossa] @f273_typedThrows_callGeneric : $@convention(thin) () -> () {
+// CHECK:        bb0:
+// CHECK:          // function_ref produceBad
+// CHECK:          [[PROD:%[^,]+]] = function_ref @produceBad : $@convention(thin) () -> Bad
+// CHECK:          [[ARG1:%[^,]+]] = apply [[PROD]]() : $@convention(thin) () -> Bad
+// CHECK:          [[ARG2:%[^,]+]] = apply [[PROD]]() : $@convention(thin) () -> Bad
+// CHECK:          [[ARG1_IN:%[^,]+]] = alloc_stack $Bad
+// CHECK:          store [[ARG1]] to [trivial] [[ARG1_IN]] : $*Bad
+// CHECK:          [[ARG2_IN:%[^,]+]] = alloc_stack $Bad
+// CHECK:          store [[ARG2]] to [trivial] [[ARG2_IN]] : $*Bad
+// CHECK:          [[RESULT_OUT:%[^,]+]] = alloc_stack $Bad
+// CHECK:          [[ERR_OUT:%[^,]+]] = alloc_stack $Bad
+// CHECK:          try_apply {{.*}}<Bad, Bad>([[RESULT_OUT]], [[ERR_OUT]], [[ARG1_IN]], [[ARG2_IN]]) : $@convention(thin) <τ_0_0, τ_0_1 where τ_0_0 : Error> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_1) -> (@out τ_0_1, @error_indirect τ_0_0), normal bb2, error bb1
+
+// CHECK:        bb1:
+// CHECK:          [[ERR:%[^,]+]] = load [trivial] [[ERR_OUT]] : $*Bad
+// CHECK:          dealloc_stack [[ERR_OUT]] : $*Bad
+// CHECK:          dealloc_stack [[RESULT_OUT]] : $*Bad
+// CHECK:          dealloc_stack [[ARG2_IN]] : $*Bad
+// CHECK:          dealloc_stack [[ARG1_IN]] : $*Bad
+// CHECK:          ignored_use [[ERR]] : $Bad
+// CHECK:          ignored_use [[ERR]] : $Bad
+// CHECK:          br bb3
+
+// CHECK:        bb2(%11 : $()):
+// CHECK:          dealloc_stack [[ERR_OUT]] : $*Bad
+// CHECK:          [[RESULT:%[^,]+]] = load [trivial] [[RESULT_OUT]] : $*Bad
+// CHECK:          dealloc_stack [[RESULT_OUT]] : $*Bad
+// CHECK:          dealloc_stack [[ARG2_IN]] : $*Bad
+// CHECK:          dealloc_stack [[ARG1_IN]] : $*Bad
+// CHECK:          ignored_use [[RESULT]] : $Bad
+// CHECK:          br bb3
+
+// CHECK:        bb3:
+// CHECK:          return {{.*}} : $()
+// CHECK-LABEL:  } // end sil function 'f273_typedThrows_callGeneric'
+sil [ossa] @f273_typedThrows_callGeneric : $@convention(thin) () -> () {
+bb0:
+  %fn = function_ref @produceBad : $@convention(thin) () -> Bad
+  %arg1 = apply %fn() : $@convention(thin) () -> Bad
+  %arg2 = apply %fn() : $@convention(thin) () -> Bad
+  %callee = function_ref @f272_typedThrows_throwGeneric : $@convention(thin) <τ_0_0, τ_0_1 where τ_0_0 : Error> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_1) -> (@out τ_0_1, @error_indirect τ_0_0)
+  try_apply %callee<Bad, Bad>(%arg1, %arg2) : $@convention(thin) <τ_0_0, τ_0_1 where τ_0_0 : Error> (@in_guaranteed τ_0_0, @in_guaranteed τ_0_1) -> (@out τ_0_1, @error_indirect τ_0_0), normal normalBB, error errorBB
+
+normalBB(%r : $Bad):
+  ignored_use %r
+  br exitBB
+
+errorBB(%e : $Bad):
+  ignored_use %e
+  ignored_use %e
+  br exitBB
+
+exitBB:
+  %t = tuple ()
+  return %t
+}
+
+
 // CHECK-LABEL: sil [ossa] @fixeeLifetime : {{.*}} {
 // CHECK:       {{bb[0-9]+}}([[ADDR:%[^,]+]] : $*T):
 // CHECK:         fix_lifetime [[ADDR]]