Skip to content

SILOptimizer: fix partial_apply optimization. #31552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions lib/SILOptimizer/IPO/CapturePropagation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -460,15 +460,20 @@ bool CapturePropagation::optimizePartialApply(PartialApplyInst *PAI) {
// arguments are dead?
std::pair<SILFunction *, SILFunction *> GenericSpecialized;
SILOptFunctionBuilder FuncBuilder(*this);
if (auto *NewFunc = getSpecializedWithDeadParams(FuncBuilder,
PAI, SubstF, PAI->getNumArguments(), GenericSpecialized)) {
rewritePartialApply(PAI, NewFunc);
if (GenericSpecialized.first) {
// Notify the pass manager about the new function.
addFunctionToPassManagerWorklist(GenericSpecialized.first,
GenericSpecialized.second);
if (auto *NewFunc = getSpecializedWithDeadParams(FuncBuilder, PAI, SubstF,
PAI->getNumArguments(),
GenericSpecialized)) {
// `partial_apply` can be rewritten to `thin_to_thick_function` only if the
// specialized callee is `@convention(thin)`.
if (NewFunc->getRepresentation() == SILFunctionTypeRepresentation::Thin) {
rewritePartialApply(PAI, NewFunc);
if (GenericSpecialized.first) {
// Notify the pass manager about the new function.
addFunctionToPassManagerWorklist(GenericSpecialized.first,
GenericSpecialized.second);
}
return true;
}
return true;
}

// Second possibility: Are all partially applied arguments constant?
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// RUN: %target-build-swift -Osize %s
// REQUIRES: asserts

// SR-12732: Fix `partial_apply` optimization.

// Do not rewrite `partial_apply` to `thin_to_thick_function` if the specialized
// callee is not `@convention(thin)`.

import DifferentiationUnittest
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to remove the dependency on DifferentiationUnittest. I tried but didn't quite figure it out: -enable-library-evolution seems involved, but passing it exposes another bug (TF-1274).

Let's visit this later to unblock progress.


func callback(_ x: inout Tracked<Float>.TangentVector) {}

@differentiable
func caller(_ x: Tracked<Float>) -> Tracked<Float> {
return x.withDerivative(callback)
}

// SIL verification failed: operand of thin_to_thick_function must be thin: opFTy->getRepresentation() == SILFunctionType::Representation::Thin
// Verifying instruction:
// // function_ref specialized Differentiable._vjpWithDerivative(_:)
// %10 = function_ref @$s16_Differentiation14DifferentiablePAAE18_vjpWithDerivativeyx5value_13TangentVectorQzAGc8pullbacktyAGzcF0A8Unittest7TrackedVySfG_Tg5 : $@convention(method) (@guaranteed @callee_guaranteed @substituted <τ_0_0> (@inout τ_0_0) -> () for <Tracked<Float>>, @in_guaranteed Tracked<Float>) -> (@out Tracked<Float>, @owned @callee_guaranteed @substituted <τ_0_0, τ_0_1> (@in_guaranteed τ_0_0) -> @out τ_0_1 for <Tracked<Float>, Tracked<Float>>) // user: %11
// -> %11 = thin_to_thick_function %10 : $@convention(method) (@guaranteed @callee_guaranteed @substituted <τ_0_0> (@inout τ_0_0) -> () for <Tracked<Float>>, @in_guaranteed Tracked<Float>) -> (@out Tracked<Float>, @owned @callee_guaranteed @substituted <τ_0_0, τ_0_1> (@in_guaranteed τ_0_0) -> @out τ_0_1 for <Tracked<Float>, Tracked<Float>>) to $@callee_guaranteed (@guaranteed @callee_guaranteed @substituted <τ_0_0> (@inout τ_0_0) -> () for <Tracked<Float>>, @in_guaranteed Tracked<Float>) -> (@out Tracked<Float>, @owned @callee_guaranteed @substituted <τ_0_0, τ_0_1> (@in_guaranteed τ_0_0) -> @out τ_0_1 for <Tracked<Float>, Tracked<Float>>) // user: %12
2 changes: 0 additions & 2 deletions test/AutoDiff/stdlib/derivative_customization.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// RUN: %target-run-simple-swift
// REQUIRES: executable_test

// REQUIRES: SR12732

import DifferentiationUnittest
import StdlibUnittest

Expand Down
2 changes: 0 additions & 2 deletions test/AutoDiff/validation-test/custom_derivatives.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// RUN: %target-run-simple-swift
// REQUIRES: executable_test

// REQUIRES: SR12732

import StdlibUnittest
#if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
import Darwin.C
Expand Down
2 changes: 0 additions & 2 deletions test/AutoDiff/validation-test/derivative_registration.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// RUN: %target-run-simple-swift
// REQUIRES: executable_test

// REQUIRES: SR12732

import StdlibUnittest
import DifferentiationUnittest

Expand Down