From 06f52182c47f54a116777e7d2c0b71ef6ef275fd Mon Sep 17 00:00:00 2001 From: Dan Zheng Date: Wed, 8 Apr 2020 04:06:38 -0700 Subject: [PATCH 1/2] Add negative test for SR-12548. Test SR-12548: `SILCombiner::visitPartialApplyInst` rewrites `partial_apply` with `@convention(method)` callee to `thin_to_thick_function`. This produces a SIL verification error: `thin_to_thick_function` only supports `@convention(thin)` operands. --- ...rite-partial-apply-convention-method.swift | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 test/AutoDiff/compiler_crashers/sr12548-siloptimizer-rewrite-partial-apply-convention-method.swift diff --git a/test/AutoDiff/compiler_crashers/sr12548-siloptimizer-rewrite-partial-apply-convention-method.swift b/test/AutoDiff/compiler_crashers/sr12548-siloptimizer-rewrite-partial-apply-convention-method.swift new file mode 100644 index 0000000000000..4494f9092d186 --- /dev/null +++ b/test/AutoDiff/compiler_crashers/sr12548-siloptimizer-rewrite-partial-apply-convention-method.swift @@ -0,0 +1,27 @@ +// RUN: not %target-build-swift -O %s + +// SR-12548: SIL verification error regarding +// `CapturePropagation::rewritePartialApply` for `partial_apply` with +// `@convention(method)` callee. + +import _Differentiation + +protocol Protocol: Differentiable { + @differentiable + func method() -> Self +} + +extension Protocol { + @differentiable + func method() -> Self { self } +} + +struct Struct: Protocol {} + +let _: @differentiable (Struct) -> Struct = { $0.method() } + +// SIL verification failed: operand of thin_to_thick_function must be thin: opFTy->getRepresentation() == SILFunctionType::Representation::Thin +// Verifying instruction: +// // function_ref specialized Protocol.method() +// %5 = function_ref @$s7crasher8ProtocolPAAE6methodxyFAA6StructV_TG5 : $@convention(method) (@in_guaranteed Struct) -> @out Struct // user: %6 +// -> %6 = thin_to_thick_function %5 : $@convention(method) (@in_guaranteed Struct) -> @out Struct to $@callee_guaranteed (@in_guaranteed Struct) -> @out Struct // user: %11 From 606c070e11c94c388c727ef7f55d0dee259854d3 Mon Sep 17 00:00:00 2001 From: Dan Zheng Date: Wed, 8 Apr 2020 05:15:37 -0700 Subject: [PATCH 2/2] [SILOptimizer] Disable `SILCombiner::visitPartialApplyInst` for method callees. Disable `SILCombiner::visitPartialApplyInst` from rewriting `partial_apply` with with `@convention(method)` callee to `thin_to_thick_function`. This fixes SIL verification errors: `thin_to_thick_function` only supports `@convention(thin)` operands. Resolves SR-12548. --- .../SILCombiner/SILCombinerApplyVisitors.cpp | 6 ++++-- ...ewrite-partial-apply-convention-method.swift | 2 +- test/SILOptimizer/sil_combine_apply.sil | 17 +++++++++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) rename test/AutoDiff/{compiler_crashers => compiler_crashers_fixed}/sr12548-siloptimizer-rewrite-partial-apply-convention-method.swift (96%) diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index 977a2f7dd7355..634468033871e 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -83,8 +83,10 @@ SILInstruction *SILCombiner::visitPartialApplyInst(PartialApplyInst *PAI) { return nullptr; // partial_apply without any substitutions or arguments is just a - // thin_to_thick_function. - if (!PAI->hasSubstitutions() && (PAI->getNumArguments() == 0)) { + // thin_to_thick_function. thin_to_thick_function supports only thin operands. + if (!PAI->hasSubstitutions() && (PAI->getNumArguments() == 0) && + PAI->getSubstCalleeType()->getRepresentation() == + SILFunctionTypeRepresentation::Thin) { if (!PAI->isOnStack()) return Builder.createThinToThickFunction(PAI->getLoc(), PAI->getCallee(), PAI->getType()); diff --git a/test/AutoDiff/compiler_crashers/sr12548-siloptimizer-rewrite-partial-apply-convention-method.swift b/test/AutoDiff/compiler_crashers_fixed/sr12548-siloptimizer-rewrite-partial-apply-convention-method.swift similarity index 96% rename from test/AutoDiff/compiler_crashers/sr12548-siloptimizer-rewrite-partial-apply-convention-method.swift rename to test/AutoDiff/compiler_crashers_fixed/sr12548-siloptimizer-rewrite-partial-apply-convention-method.swift index 4494f9092d186..007b4b997dfce 100644 --- a/test/AutoDiff/compiler_crashers/sr12548-siloptimizer-rewrite-partial-apply-convention-method.swift +++ b/test/AutoDiff/compiler_crashers_fixed/sr12548-siloptimizer-rewrite-partial-apply-convention-method.swift @@ -1,4 +1,4 @@ -// RUN: not %target-build-swift -O %s +// RUN: %target-build-swift -O %s // SR-12548: SIL verification error regarding // `CapturePropagation::rewritePartialApply` for `partial_apply` with diff --git a/test/SILOptimizer/sil_combine_apply.sil b/test/SILOptimizer/sil_combine_apply.sil index 438af89481949..e00fbd92d6533 100644 --- a/test/SILOptimizer/sil_combine_apply.sil +++ b/test/SILOptimizer/sil_combine_apply.sil @@ -974,3 +974,20 @@ bb0(%0: $*FakeProtocol): %r = tuple () return %r : $() } + +// Test `partial_apply` with a `@convention(method)` callee. +// Rewriting `partial_apply` to `thin_to_thick_function` is not okay because +// `thin_to_thick_function` supports only `@convention(thin)` operands. + +sil @method : $@convention(method) (Int32) -> () + +sil @test_partial_apply_method : $@convention(thin) () -> @owned @callee_owned (Int32) -> () { + %1 = function_ref @method : $@convention(method) (Int32) -> () + %2 = partial_apply %1() : $@convention(method) (Int32) -> () + return %2 : $@callee_owned (Int32) -> () +} + +// CHECK-LABEL: sil @test_partial_apply_method +// CHECK: [[FN:%.*]] = function_ref @method +// CHECK-NEXT: partial_apply [[FN]]() +// CHECK-NEXT: return