Merge pull request #58673 from jckarter/closure-literal-function-conversion-throws-5.7

[5.7] SILGen: Emit a closure literal in a function conversion as the converted type.

Author: jckarter

lib/IRGen/LoadableByAddress.cpp

@@ -1894,8 +1894,8 @@ static void allocateAndSetAll(StructLoweringState &pass,
   }
 }
 
-static void castTupleInstr(SingleValueInstruction *instr, IRGenModule &Mod,
-                           LargeSILTypeMapper &Mapper) {
+static void retypeTupleInstr(SingleValueInstruction *instr, IRGenModule &Mod,
+                             LargeSILTypeMapper &Mapper) {
   SILType currSILType = instr->getType();
   auto funcType = getInnerFunctionType(currSILType);
   assert(funcType && "Expected a function Type");
@@ -1910,25 +1910,31 @@ static void castTupleInstr(SingleValueInstruction *instr, IRGenModule &Mod,
 
   auto II = instr->getIterator();
   ++II;
-  SILBuilderWithScope castBuilder(II);
-  SingleValueInstruction *castInstr = nullptr;
+  SILBuilderWithScope Builder(II);
+  SingleValueInstruction *newInstr = nullptr;
   switch (instr->getKind()) {
   // Add cast to the new sil function type:
   case SILInstructionKind::TupleExtractInst: {
-    castInstr = castBuilder.createUncheckedReinterpretCast(
-        instr->getLoc(), instr, newSILType.getObjectType());
+    auto extractInst = cast<TupleExtractInst>(instr);
+    newInstr = Builder.createTupleExtract(
+      extractInst->getLoc(), extractInst->getOperand(),
+      extractInst->getFieldIndex(),
+      newSILType.getObjectType());
     break;
   }
   case SILInstructionKind::TupleElementAddrInst: {
-    castInstr = castBuilder.createUncheckedAddrCast(
-        instr->getLoc(), instr, newSILType.getAddressType());
+    auto elementAddrInst = cast<TupleElementAddrInst>(instr);
+    newInstr = Builder.createTupleElementAddr(
+      elementAddrInst->getLoc(), elementAddrInst->getOperand(),
+      elementAddrInst->getFieldIndex(),
+      newSILType.getAddressType());
     break;
   }
   default:
     llvm_unreachable("Unexpected instruction inside tupleInstsToMod");
   }
-  instr->replaceAllUsesWith(castInstr);
-  castInstr->setOperand(0, instr);
+  instr->replaceAllUsesWith(newInstr);
+  instr->eraseFromParent();
 }
 
 static SILValue createCopyOfEnum(StructLoweringState &pass,
@@ -2090,7 +2096,7 @@ static void rewriteFunction(StructLoweringState &pass,
   }
 
   for (SingleValueInstruction *instr : pass.tupleInstsToMod) {
-    castTupleInstr(instr, pass.Mod, pass.Mapper);
+    retypeTupleInstr(instr, pass.Mod, pass.Mapper);
   }
 
   while (!pass.allocStackInstsToMod.empty()) {

lib/SIL/IR/SILBuilder.cpp

@@ -154,6 +154,18 @@ SILBuilder::createUncheckedReinterpretCast(SILLocation Loc, SILValue Op,
   if (SILType::canRefCast(Op->getType(), Ty, getModule()))
     return createUncheckedRefCast(Loc, Op, Ty);
 
+  // If the source and destination types are functions with the same
+  // kind of representation, then do a function conversion.
+  if (Op->getType().isObject() && Ty.isObject()) {
+    if (auto OpFnTy = Op->getType().getAs<SILFunctionType>()) {
+      if (auto DestFnTy = Ty.getAs<SILFunctionType>()) {
+        if (OpFnTy->getRepresentation() == DestFnTy->getRepresentation()) {
+          return createConvertFunction(Loc, Op, Ty, /*withoutActuallyEscaping*/ false);
+        }
+      }
+    }
+  }
+  
   // The destination type is nontrivial, and may be smaller than the source
   // type, so RC identity cannot be assumed.
   return insert(UncheckedBitwiseCastInst::create(
@@ -175,6 +187,18 @@ SILBuilder::createUncheckedBitCast(SILLocation Loc, SILValue Op, SILType Ty) {
   if (SILType::canRefCast(Op->getType(), Ty, getModule()))
     return createUncheckedRefCast(Loc, Op, Ty);
 
+  // If the source and destination types are functions with the same
+  // kind of representation, then do a function conversion.
+  if (Op->getType().isObject() && Ty.isObject()) {
+    if (auto OpFnTy = Op->getType().getAs<SILFunctionType>()) {
+      if (auto DestFnTy = Ty.getAs<SILFunctionType>()) {
+        if (OpFnTy->getRepresentation() == DestFnTy->getRepresentation()) {
+          return createConvertFunction(Loc, Op, Ty, /*withoutActuallyEscaping*/ false);
+        }
+      }
+    }
+  }
+  
   // The destination type is nontrivial, and may be smaller than the source
   // type, so RC identity cannot be assumed.
   return createUncheckedValueCast(Loc, Op, Ty);

lib/SILGen/SILGenExpr.cpp

@@ -1714,6 +1714,26 @@ static ManagedValue convertFunctionRepresentation(SILGenFunction &SGF,
   llvm_unreachable("bad representation");
 }
 
+// Ideally our prolog/epilog emission would be able to handle all possible
+// reabstractions and conversions. Until then, this returns true if a closure
+// literal of type `literalType` can be directly emitted by SILGen as
+// `convertedType`.
+static bool canPeepholeLiteralClosureConversion(Type literalType,
+                                                Type convertedType) {
+  auto literalFnType = literalType->getAs<FunctionType>();
+  auto convertedFnType = convertedType->getAs<FunctionType>();
+  
+  if (!literalFnType || !convertedFnType)
+    return false;
+  
+  // Does the conversion only add `throws`?
+  if (literalFnType->isEqual(convertedFnType->getWithoutThrowing())) {
+    return true;
+  }
+  
+  return false;
+}
+
 RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
                                                   SGFContext C)
 {
@@ -1748,6 +1768,38 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
     return RValue(SGF, e, result);
   }
 
+  // If the function being converted is a closure literal, then the only use
+  // of the closure should be as the destination type of the conversion. Rather
+  // than emit the closure as is and convert it, see if we can emit the closure
+  // directly as the desired type.
+  //
+  // TODO: Move this up when we can emit closures directly with C calling
+  // convention.
+  auto subExpr = e->getSubExpr()->getSemanticsProvidingExpr();
+  if (isa<AbstractClosureExpr>(subExpr)
+      && canPeepholeLiteralClosureConversion(subExpr->getType(),
+                                             e->getType())) {
+    // If we're emitting into a context with a preferred abstraction pattern
+    // already, carry that along.
+    auto origType = C.getAbstractionPattern();
+    // If not, use the conversion type as the desired abstraction pattern.
+    if (!origType) {
+      origType = AbstractionPattern(e->getType()->getCanonicalType());
+    }
+    
+    auto substType = subExpr->getType()->getCanonicalType();
+    
+    auto conversion = Conversion::getSubstToOrig(*origType, substType,
+                                      SGF.getLoweredType(*origType, substType));
+    ConvertingInitialization convertingInit(conversion, SGFContext());
+    auto closure = SGF.emitRValue(subExpr,
+                                  SGFContext(&convertingInit))
+      .getAsSingleValue(SGF, e);
+    closure = SGF.emitSubstToOrigValue(e, closure, *origType, substType);
+
+    return RValue(SGF, e, closure);
+  }
+  
   // Handle a reference to a "thin" native Swift function that only changes
   // representation and refers to an inherently thin function reference.
   if (destRepTy->getRepresentation() == FunctionTypeRepresentation::Thin) {

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

@@ -146,42 +146,20 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,
   if (SubstCalleeTy->hasArchetype() || ConvertCalleeTy->hasArchetype())
     return nullptr;
 
-  // Indirect results are not currently handled.
-  if (AI.hasIndirectSILResults())
-    return nullptr;
-
-  // Bail if the result type of the converted callee is different from the callee's
-  // result type of the apply instruction.
-  if (SubstCalleeTy->getAllResultsSubstType(
-          AI.getModule(), AI.getFunction()->getTypeExpansionContext()) !=
-      ConvertCalleeTy->getAllResultsSubstType(
-          AI.getModule(), AI.getFunction()->getTypeExpansionContext())) {
-    return nullptr;
-  }
-
   // Ok, we can now perform our transformation. Grab AI's operands and the
   // relevant types from the ConvertFunction function type and AI.
   Builder.setCurrentDebugScope(AI.getDebugScope());
-  OperandValueArrayRef Ops = AI.getArgumentsWithoutIndirectResults();
+  OperandValueArrayRef Ops = AI.getArguments();
   SILFunctionConventions substConventions(SubstCalleeTy, FRI->getModule());
   SILFunctionConventions convertConventions(ConvertCalleeTy, FRI->getModule());
   auto context = AI.getFunction()->getTypeExpansionContext();
-  auto oldOpTypes = substConventions.getParameterSILTypes(context);
-  auto newOpTypes = convertConventions.getParameterSILTypes(context);
-
-  assert(Ops.size() == SubstCalleeTy->getNumParameters()
-         && "Ops and op types must have same size.");
-  assert(Ops.size() == ConvertCalleeTy->getNumParameters()
-         && "Ops and op types must have same size.");
+  auto oldOpRetTypes = substConventions.getIndirectSILResultTypes(context);
+  auto newOpRetTypes = convertConventions.getIndirectSILResultTypes(context);
+  auto oldOpParamTypes = substConventions.getParameterSILTypes(context);
+  auto newOpParamTypes = convertConventions.getParameterSILTypes(context);
 
   llvm::SmallVector<SILValue, 8> Args;
-  auto newOpI = newOpTypes.begin();
-  auto oldOpI = oldOpTypes.begin();
-  for (unsigned i = 0, e = Ops.size(); i != e; ++i, ++newOpI, ++oldOpI) {
-    SILValue Op = Ops[i];
-    SILType OldOpType = *oldOpI;
-    SILType NewOpType = *newOpI;
-
+  auto convertOp = [&](SILValue Op, SILType OldOpType, SILType NewOpType) {
     // Convert function takes refs to refs, address to addresses, and leaves
     // other types alone.
     if (OldOpType.isAddress()) {
@@ -190,17 +168,68 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,
       Args.push_back(UAC);
     } else if (OldOpType.getASTType() != NewOpType.getASTType()) {
       auto URC =
-          Builder.createUncheckedReinterpretCast(AI.getLoc(), Op, NewOpType);
+          Builder.createUncheckedBitCast(AI.getLoc(), Op, NewOpType);
       Args.push_back(URC);
     } else {
       Args.push_back(Op);
     }
+  };
+
+  unsigned OpI = 0;
+  
+  auto newRetI = newOpRetTypes.begin();
+  auto oldRetI = oldOpRetTypes.begin();
+  
+  for (auto e = newOpRetTypes.end(); newRetI != e;
+       ++OpI, ++newRetI, ++oldRetI) {
+    convertOp(Ops[OpI], *oldRetI, *newRetI);
+  }
+  
+  auto newParamI = newOpParamTypes.begin();
+  auto oldParamI = oldOpParamTypes.begin();
+  for (auto e = newOpParamTypes.end(); newParamI != e;
+       ++OpI, ++newParamI, ++oldParamI) {
+    convertOp(Ops[OpI], *oldParamI, *newParamI);
   }
 
+  // Convert the direct results if they changed.
+  auto oldResultTy = SubstCalleeTy
+    ->getDirectFormalResultsType(AI.getModule(),
+                                 AI.getFunction()->getTypeExpansionContext());
+  auto newResultTy = ConvertCalleeTy
+    ->getDirectFormalResultsType(AI.getModule(),
+                                 AI.getFunction()->getTypeExpansionContext());
+  
   // Create the new apply inst.
   if (auto *TAI = dyn_cast<TryApplyInst>(AI)) {
+    // If the results need to change, create a new landing block to do that
+    // conversion.
+    auto normalBB = TAI->getNormalBB();
+    if (oldResultTy != newResultTy) {
+      normalBB = AI.getFunction()->createBasicBlockBefore(TAI->getNormalBB());
+      Builder.setInsertionPoint(normalBB);
+      SmallVector<SILValue, 4> branchArgs;
+      
+      auto oldOpResultTypes = substConventions.getDirectSILResultTypes(context);
+      auto newOpResultTypes = convertConventions.getDirectSILResultTypes(context);
+      
+      auto oldRetI = oldOpResultTypes.begin();
+      auto newRetI = newOpResultTypes.begin();
+      auto origArgs = TAI->getNormalBB()->getArguments();
+      auto origArgI = origArgs.begin();
+      for (auto e = newOpResultTypes.end(); newRetI != e;
+           ++oldRetI, ++newRetI, ++origArgI) {
+        auto arg = normalBB->createPhiArgument(*newRetI, (*origArgI)->getOwnershipKind());
+        auto converted = Builder.createUncheckedBitCast(AI.getLoc(),
+                                                                arg, *oldRetI);
+        branchArgs.push_back(converted);
+      }
+      
+      Builder.createBranch(AI.getLoc(), TAI->getNormalBB(), branchArgs);
+    }
+    
     return Builder.createTryApply(AI.getLoc(), FRI, SubstitutionMap(), Args,
-                                  TAI->getNormalBB(), TAI->getErrorBB(),
+                                  normalBB, TAI->getErrorBB(),
                                   TAI->getApplyOptions());
   }
 
@@ -213,12 +242,13 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI,
     Options |= ApplyFlags::DoesNotThrow;
   ApplyInst *NAI = Builder.createApply(AI.getLoc(), FRI, SubstitutionMap(),
                                        Args, Options);
-  assert(FullApplySite(NAI).getSubstCalleeType()->getAllResultsSubstType(
-             AI.getModule(), AI.getFunction()->getTypeExpansionContext()) ==
-             AI.getSubstCalleeType()->getAllResultsSubstType(
-                 AI.getModule(), AI.getFunction()->getTypeExpansionContext()) &&
-         "Function types should be the same");
-  return NAI;
+  SILInstruction *result = NAI;
+  
+  if (oldResultTy != newResultTy) {
+    result = Builder.createUncheckedBitCast(AI.getLoc(), NAI, oldResultTy);
+  }
+  
+  return result;
 }
 
 /// Try to optimize a keypath application with an apply instruction.

test/AutoDiff/SILOptimizer/activity_analysis.swift

@@ -552,14 +552,13 @@ func testTryApply(_ x: Float) -> Float {
 // CHECK: bb0:
 // CHECK: [ACTIVE] %0 = argument of bb0 : $Float
 // CHECK: [NONE]   // function_ref closure #1 in testTryApply(_:)
-// CHECK: [NONE]   %3 = convert_function %2 : $@convention(thin) () -> () to $@convention(thin) @noescape () -> ()
-// CHECK: [NONE]   %4 = thin_to_thick_function %3 : $@convention(thin) @noescape () -> () to $@noescape @callee_guaranteed () -> ()
-// CHECK: [NONE]   %5 = convert_function %4 : $@noescape @callee_guaranteed () -> () to $@noescape @callee_guaranteed () -> @error Error
+// CHECK: [NONE]   %3 = convert_function %2 :
+// CHECK: [NONE]   %4 = thin_to_thick_function %3 :
 // CHECK: [NONE]   // function_ref rethrowing(_:)
 // CHECK: bb1:
-// CHECK: [NONE] %8 = argument of bb1 : $()
+// CHECK: [NONE] %7 = argument of bb1 : $()
 // CHECK: bb2:
-// CHECK: [NONE] %10 = argument of bb2 : $Error
+// CHECK: [NONE] %9 = argument of bb2 : $Error
 
 //===----------------------------------------------------------------------===//
 // Coroutine differentiation (`begin_apply`)

test/DebugInfo/patternvars.swift

@@ -35,7 +35,7 @@ public func mangle(s: [UnicodeScalar]) -> [UnicodeScalar] {
 // Do we care to expose these via lldb?
 
 // CHECK: define {{.*}}@"$s11patternvars6mangle1sSayAA13UnicodeScalarVGAF_tFA2EXEfU_"
-// CHECK: %[[VAL:[0-9]+]] = call swiftcc i32 @"$s11patternvars13UnicodeScalarV5values6UInt32Vvg"(i32 %0)
+// CHECK: %[[VAL:[0-9]+]] = call swiftcc i32 @"$s11patternvars13UnicodeScalarV5values6UInt32Vvg"(
 // CHECK:       {{[0-9]+}}:
 // CHECK-NOT:   call void @llvm.dbg.value
 // CHECK-NOT:   call void asm sideeffect "", "r"

test/IRGen/big_types_corner_cases.swift

@@ -203,7 +203,7 @@ public func testGetFunc() {
 // CHECK-LABEL: define{{( dllexport)?}}{{( protected)?}} hidden swiftcc void @"$s22big_types_corner_cases7TestBigC4testyyF"(%T22big_types_corner_cases7TestBigC* swiftself %0)
 // CHECK: [[CALL1:%.*]] = call {{.*}} @__swift_instantiateConcreteTypeFromMangledName({{.*}} @"$sSayy22big_types_corner_cases9BigStructVcSgGMD"
 // CHECK: [[CALL2:%.*]] = call i8** @"$sSayy22big_types_corner_cases9BigStructVcSgGSayxGSlsWl
-// CHECK:  call swiftcc void @"$sSlsE10firstIndex5where0B0QzSgSb7ElementQzKXE_tKF"(%swift.opaque* noalias nocapture sret({{.*}}) %{{[0-9]+}}, i8* bitcast ({{.*}}* @"$s22big_types_corner_cases9BigStruct{{.*}}_TRTA{{(\.ptrauth)?}}" to i8*), %swift.opaque* %{{[0-9]+}}, %swift.type* %{{[0-9]+}}, i8** [[CALL2]]
+// CHECK:  call swiftcc void @"$sSlsE10firstIndex5where0B0QzSgSb7ElementQzKXE_tKF"(%swift.opaque* noalias nocapture sret({{.*}}) %{{[0-9]+}}, i8* bitcast ({{.*}}* @"$s22big_types_corner_cases7TestBig{{.*}}" to i8*), %swift.opaque* null, %swift.type* %{{[0-9]+}}, i8** [[CALL2]]
 class TestBig {
     typealias Handler = (BigStruct) -> Void
 

test/Profiler/coverage_closure_returns_never.swift

@@ -2,7 +2,8 @@
 
 // CHECK-LABEL: closure #1 (Swift.Never) -> Swift.Never in coverage_closure_returns_never.closure_with_fatal_error(Swift.Array<Swift.Never>) -> ()
 // CHECK: builtin "int_instrprof_increment"
-// CHECK-NEXT: debug_value {{.*}} : $Never
+// CHECK-NEXT: [[LOAD:%.*]] = load {{.*}} : $*Never
+// CHECK-NEXT: debug_value [[LOAD]] : $Never
 // CHECK-NEXT: unreachable
 
 func closure_with_fatal_error(_ arr: [Never]) {

test/SILGen/closure_literal_reabstraction_async.swift

@@ -0,0 +1,17 @@
+// RUN: %target-swift-frontend -emit-silgen %s -disable-availability-checking | %FileCheck %s
+// REQUIRES: concurrency
+
+@_silgen_name("takeThrowingAsyncClosure")
+func takeThrowingAsyncClosure<T>(_: () async throws -> T) 
+
+// CHECK-LABEL: sil {{.*}} @{{.*}}34passNonthrowingAsyncClosureLiteral
+func passNonthrowingAsyncClosureLiteral() {
+    // Check that the literal closure was emitted directly with an error return,
+    // without a reabstraction thunk to convert from nonthrowing.
+    // CHECK: [[INVOKE_FN:%.*]] = function_ref
+    // CHECK: [[CONVERT_NOESCAPE:%.*]] = convert_function [[INVOKE_FN]]
+    // CHECK: [[CLOSURE:%.*]] = thin_to_thick_function [[CONVERT_NOESCAPE]]
+    // CHECK: [[CALLEE:%.*]] = function_ref @takeThrowingAsyncClosure
+    // CHECK: apply [[CALLEE]]<Int>([[CLOSURE]])
+    takeThrowingAsyncClosure { return 42 }
+}

test/SILGen/rethrows.swift

@@ -71,12 +71,11 @@ func test2() {
 }
 
 // CHECK-LABEL: sil hidden @$s8rethrows5test3yyF : $@convention(thin) () -> () {
-// CHECK:       [[CLOSURE:%.*]] = function_ref @$s8rethrows5test3yyFSiyXEfU_ : $@convention(thin) () -> Int
-// CHECK:       [[CVT:%.*]] = convert_function [[CLOSURE]] : $@convention(thin) () -> Int to $@convention(thin) @noescape () -> Int
+// CHECK:       [[CLOSURE:%.*]] = function_ref @$s8rethrows5test3yyFSiyXEfU_ :
+// CHECK:       [[CVT:%.*]] = convert_function [[CLOSURE]] :
 // CHECK:       [[T0:%.*]] = thin_to_thick_function [[CVT]]
-// CHECK:       [[T1:%.*]] = convert_function [[T0]] : $@noescape @callee_guaranteed () -> Int to $@noescape @callee_guaranteed () -> (Int, @error Error)
-// CHECK:       [[RETHROWER:%.*]] = function_ref @$s8rethrows9rethroweryS2iyKXEKF : $@convention(thin) (@noescape @callee_guaranteed () -> (Int, @error Error)) -> (Int, @error Error)
-// CHECK:       try_apply [[RETHROWER]]([[T1]]) : $@convention(thin) (@noescape @callee_guaranteed () -> (Int, @error Error)) -> (Int, @error Error), normal [[NORMAL:bb1]], error [[ERROR:bb2]]
+// CHECK:       [[RETHROWER:%.*]] = function_ref @$s8rethrows9rethroweryS2iyKXEKF :
+// CHECK:       try_apply [[RETHROWER]]([[T0]]) :
 // CHECK:     [[NORMAL]]({{%.*}} : $Int):
 // CHECK-NEXT:  [[RESULT:%.*]] = tuple ()
 // CHECK-NEXT:  return [[RESULT]]