[AutoDiff] Fix two derivative type calculation bugs caught by RequirementMachine #39505

rxwei · 2021-09-29T12:53:38Z

When calculating the differential type of an original function with an inout parameter and when the inout parameter has a type parameter, the inout parameter should get a generic parameter in the subst generic signature of the differential but it currently doesn't. This causes SILGen to attempt to reabstract the differential value in the JVP protocol witness thunk, whilst the generic signature is lacking requirements, leading to a requirement machine error. This patch fixes the calculation so that the JVP's result type (the differential type) always matches the witness thunk's result type.

Wrong type:

         sil private [transparent] [thunk] [ossa] @... <τ_0_0 where τ_0_0 : Differentiable> (...) -> @owned @callee_guaranteed @Substituted <τ_0_0, τ_0_1> (@in_guaranteed τ_0_0) -> @out τ_0_1 for <τ_0_0.TangentVector, τ_0_0.TangentVector> {
           %6 = differentiable_function_extract [jvp] %5 : $@differentiable(reverse) @convention(method) <τ_0_0 where τ_0_0 : Differentiable> (@in_guaranteed τ_0_0, @noDerivative @inout τ_0_0, @noDerivative SR_13305_Struct) -> () // user: %7
HERE ====> %7 = apply %6<τ_0_0>(%0, %1, %3) : $@convention(method) <τ_0_0 where τ_0_0 : Differentiable> (@in_guaranteed τ_0_0, @inout τ_0_0, SR_13305_Struct) -> @owned @callee_guaranteed @Substituted <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0 for <τ_0_0.TangentVector>

Should be:

  %7 = apply %6<τ_0_0>(%0, %1, %3) : $@convention(method) <τ_0_0 where τ_0_0 : Differentiable> (@in_guaranteed τ_0_0, @inout τ_0_0, SR_13305_Struct) -> @owned @callee_guaranteed @Substituted <τ_0_0, τ_0_1> (@in_guaranteed τ_0_0) -> @out τ_0_1 for <τ_0_0.TangentVector, τ_0_0.TangentVector>

TypeConverter::makeConstantInterfaceType is not passing down the derivative generic signature to SILFunctionType::getAutoDiffDerivativeFunctionType for class methods, and this was caught by RequirementMachine during vtable emission. This patch fixes that.

Partially resolves rdar://82549134. The only remaining tests that require -requirement-machine=off are SILOptimizer/semantic_member_accessors_sil.swift and SILOptimizer/differentiation_diagnostics.swift which I will fix next. Then I'll do a proper fix for workaround #39416.

rxwei · 2021-09-29T12:54:12Z

@swift-ci please test

rxwei · 2021-09-29T12:59:52Z

This also fixes a compiler crasher unrelated to RequirementMachine:

import _Differentiation

protocol P {
  @differentiable(reverse, wrt: x)
  func genericMethod<T: Differentiable, U: Differentiable>(_ x: T, _ y: inout U)
}

struct S: P {
  func genericMethod<T: Differentiable, U: Differentiable>(_ x: T, _ y: inout U) {
  }
}

Assertion failed: (!param.getInterfaceType()->hasError() && "interface type of parameter should not contain error types"), function SILFunctionType, file ASTContext.cpp, line 3854.
...
7  swift-frontend           0x00000001079684b0 swift::SILFunctionType::SILFunctionType(swift::GenericSignature, swift::SILExtInfo, swift::SILCoroutineKind, swift::ParameterConvention, llvm::ArrayRef<swift::SILParameterInfo>, llvm::ArrayRef<swift::SILYieldInfo>, llvm::ArrayRef<swift::SILResultInfo>, llvm::Optional<swift::SILResultInfo>, swift::SubstitutionMap, swift::SubstitutionMap, swift::ASTContext const&, swift::RecursiveTypeProperties, swift::ProtocolConformanceRef) + 2244
8  swift-frontend           0x0000000107969528 swift::SILFunctionType::SILFunctionType(swift::GenericSignature, swift::SILExtInfo, swift::SILCoroutineKind, swift::ParameterConvention, llvm::ArrayRef<swift::SILParameterInfo>, llvm::ArrayRef<swift::SILYieldInfo>, llvm::ArrayRef<swift::SILResultInfo>, llvm::Optional<swift::SILResultInfo>, swift::SubstitutionMap, swift::SubstitutionMap, swift::ASTContext const&, swift::RecursiveTypeProperties, swift::ProtocolConformanceRef) + 248
9  swift-frontend           0x000000010796a17c swift::SILFunctionType::get(swift::GenericSignature, swift::SILExtInfo, swift::SILCoroutineKind, swift::ParameterConvention, llvm::ArrayRef<swift::SILParameterInfo>, llvm::ArrayRef<swift::SILYieldInfo>, llvm::ArrayRef<swift::SILResultInfo>, llvm::Optional<swift::SILResultInfo>, swift::SubstitutionMap, swift::SubstitutionMap, swift::ASTContext const&, swift::ProtocolConformanceRef) + 2044
10 swift-frontend           0x0000000106110ee8 swift::Lowering::SILGenFunction::buildThunkType(swift::CanTypeWrapper<swift::SILFunctionType>&, swift::CanTypeWrapper<swift::SILFunctionType>&, swift::CanType&, swift::CanType&, swift::GenericEnvironment*&, swift::SubstitutionMap&, swift::CanType&, bool) + 2484
11 swift-frontend           0x000000010613b0ac createThunk(swift::Lowering::SILGenFunction&, swift::SILLocation, swift::Lowering::ManagedValue, swift::Lowering::AbstractionPattern, swift::CanTypeWrapper<swift::AnyFunctionType>, swift::Lowering::AbstractionPattern, swift::CanTypeWrapper<swift::AnyFunctionType>, swift::Lowering::TypeLowering const&) + 1000
12 swift-frontend           0x00000001061395b4 (anonymous namespace)::Transform::transformFunction(swift::Lowering::ManagedValue, swift::Lowering::AbstractionPattern, swift::CanTypeWrapper<swift::AnyFunctionType>, swift::Lowering::AbstractionPattern, swift::CanTypeWrapper<swift::AnyFunctionType>, swift::Lowering::TypeLowering const&) + 732
13 swift-frontend           0x0000000106117414 (anonymous namespace)::Transform::transform(swift::Lowering::ManagedValue, swift::Lowering::AbstractionPattern, swift::CanType, swift::Lowering::AbstractionPattern, swift::CanType, swift::SILType, swift::Lowering::SGFContext) + 1408
14 swift-frontend           0x0000000106116310 swift::Lowering::SILGenFunction::emitTransformedValue(swift::SILLocation, swift::Lowering::ManagedValue, swift::Lowering::AbstractionPattern, swift::CanType, swift::Lowering::AbstractionPattern, swift::CanType, swift::SILType, swift::Lowering::SGFContext) + 244
15 swift-frontend           0x0000000106147b60 (anonymous namespace)::ResultPlanner::execute(llvm::ArrayRef<swift::SILValue>, llvm::SmallVectorImpl<swift::SILValue>&)::$_22::operator()((anonymous namespace)::ResultPlanner::Operation&, bool, bool) const + 580
...

swift-ci · 2021-09-29T14:13:01Z

Build failed
Swift Test Linux Platform
Git Sha - 51d82dc1a24954ac604d75ef5a549611d21d3268

swift-ci · 2021-09-29T14:32:40Z

Build failed
Swift Test OS X Platform
Git Sha - 51d82dc1a24954ac604d75ef5a549611d21d3268

test/AutoDiff/validation-test/inout_parameters.swift

slavapestov

Thank you!

rxwei · 2021-09-29T18:12:12Z

@swift-ci please test

swift-ci · 2021-09-29T19:25:15Z

Build failed
Swift Test Linux Platform
Git Sha - 552d4625c767576d9476289305f584abbf1ae72e

swift-ci · 2021-09-29T20:11:25Z

Build failed
Swift Test OS X Platform
Git Sha - 552d4625c767576d9476289305f584abbf1ae72e

BradLarson

I think I've seen that crasher before, it'll be great to have that fixed.

rxwei · 2021-09-29T21:53:03Z

@swift-ci please test

swift-ci · 2021-09-29T23:20:33Z

Build failed
Swift Test Linux Platform
Git Sha - 7f4a8b9ec26a0eebdcf9a439678ab47dc3dad986

rxwei · 2021-09-29T23:39:35Z

@swift-ci please test Linux

swift-ci · 2021-09-29T23:43:16Z

Build failed
Swift Test OS X Platform
Git Sha - 7f4a8b9ec26a0eebdcf9a439678ab47dc3dad986

swift-ci · 2021-09-29T23:57:43Z

Build failed
Swift Test Linux Platform
Git Sha - 7f4a8b9ec26a0eebdcf9a439678ab47dc3dad986

rxwei · 2021-09-30T08:54:21Z

@swift-ci please test

swift-ci · 2021-09-30T09:09:15Z

Build failed
Swift Test OS X Platform
Git Sha - 56da05a66070a0ada9af20a6d777b68278b90092

swift-ci · 2021-09-30T09:25:29Z

Build failed
Swift Test Linux Platform
Git Sha - 56da05a66070a0ada9af20a6d777b68278b90092

rxwei · 2021-09-30T19:02:13Z

@swift-ci please test and merge

@owned

…mentMachine. 1. When calculating the differential type of an original function with an inout parameter and when the inout parameter has a type parameter, the inout parameter should get a generic parameter in the subst generic signature of the differential but it currently doesn't. This causes SILGen to attempt to reabstract the differential value in the JVP protocol witness thunk, whilst the generic signature is lacking requirements, leading to a requirement machine error. This patch fixes the calculation so that the JVP's result type (the differential type) always matches the witness thunk's result type. Wrong type: ```swift sil private [transparent] [thunk] [ossa] @... <τ_0_0 where τ_0_0 : Differentiable> (...) -> @owned @callee_guaranteed @Substituted <τ_0_0, τ_0_1> (@in_guaranteed τ_0_0) -> @out τ_0_1 for <τ_0_0.TangentVector, τ_0_0.TangentVector> { %6 = differentiable_function_extract [jvp] %5 : $@differentiable(reverse) @convention(method) <τ_0_0 where τ_0_0 : Differentiable> (@in_guaranteed τ_0_0, @noDerivative @inout τ_0_0, @noDerivative SR_13305_Struct) -> () // user: %7 HERE ====> %7 = apply %6<τ_0_0>(%0, %1, %3) : $@convention(method) <τ_0_0 where τ_0_0 : Differentiable> (@in_guaranteed τ_0_0, @inout τ_0_0, SR_13305_Struct) -> @owned @callee_guaranteed @Substituted <τ_0_0> (@in_guaranteed τ_0_0) -> @out τ_0_0 for <τ_0_0.TangentVector> ``` Should be: ```swift %7 = apply %6<τ_0_0>(%0, %1, %3) : $@convention(method) <τ_0_0 where τ_0_0 : Differentiable> (@in_guaranteed τ_0_0, @inout τ_0_0, SR_13305_Struct) -> @owned @callee_guaranteed @Substituted <τ_0_0, τ_0_1> (@in_guaranteed τ_0_0) -> @out τ_0_1 for <τ_0_0.TangentVector, τ_0_0.TangentVector> ``` 2. `TypeConverter::makeConstantInterfaceType` is not passing down the derivative generic signature to `SILFunctionType::getAutoDiffDerivativeFunctionType` for class methods, and this was caught by RequirementMachine during vtable emission. This patch fixes that. Partially resolves rdar://82549134. The only remaining tests that require `-requirement-machine=off` are SILOptimizer/semantic_member_accessors_sil.swift and SILOptimizer/differentiation_diagnostics.swift which I will fix next. Then I'll do a proper fix for workaround swiftlang#39416.

rxwei · 2021-10-01T03:52:06Z

@swift-ci please test and merge

rxwei requested a review from slavapestov September 29, 2021 12:53

rxwei requested a review from BradLarson September 29, 2021 13:00

slavapestov reviewed Sep 29, 2021

View reviewed changes

test/AutoDiff/validation-test/inout_parameters.swift Outdated Show resolved Hide resolved

slavapestov approved these changes Sep 29, 2021

View reviewed changes

rxwei force-pushed the 82549134-autodiff-requirement-machine-fix-1 branch from 51d82dc to 552d462 Compare September 29, 2021 18:11

BradLarson approved these changes Sep 29, 2021

View reviewed changes

rxwei force-pushed the 82549134-autodiff-requirement-machine-fix-1 branch 2 times, most recently from a6c54da to 7f4a8b9 Compare September 29, 2021 21:52

rxwei changed the title ~~[AutoDiff] Fix differential type calculation for original functions with inout parameters.~~ [AutoDiff] Fix two derivative type calculation bugs caught by RequirementMachine Sep 29, 2021

rxwei force-pushed the 82549134-autodiff-requirement-machine-fix-1 branch 2 times, most recently from 975440f to 56da05a Compare September 30, 2021 08:54

rxwei force-pushed the 82549134-autodiff-requirement-machine-fix-1 branch from 56da05a to 68fd6ea Compare September 30, 2021 19:01

rxwei force-pushed the 82549134-autodiff-requirement-machine-fix-1 branch from 68fd6ea to 808c7ec Compare October 1, 2021 03:51

swift-ci merged commit d993112 into swiftlang:main Oct 1, 2021

rxwei deleted the 82549134-autodiff-requirement-machine-fix-1 branch October 5, 2021 03:33

rxwei mentioned this pull request Nov 4, 2021

Workaround for rdar://84716758 #40050

Closed

DougGregor mentioned this pull request Nov 4, 2021

Revert "[AutoDiff] Fix two derivative type calculation bugs caught by RequirementMachine" #40057

Merged

rxwei mentioned this pull request Nov 5, 2021

[AutoDiff] Plumb witness derivative generic signatures through SILGen. #40063

Merged

[AutoDiff] Fix two derivative type calculation bugs caught by RequirementMachine #39505

[AutoDiff] Fix two derivative type calculation bugs caught by RequirementMachine #39505

Uh oh!

Conversation

rxwei commented Sep 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rxwei commented Sep 29, 2021

Uh oh!

rxwei commented Sep 29, 2021

Uh oh!

swift-ci commented Sep 29, 2021

Uh oh!

swift-ci commented Sep 29, 2021

Uh oh!

Uh oh!

slavapestov left a comment

Choose a reason for hiding this comment

Uh oh!

rxwei commented Sep 29, 2021

Uh oh!

swift-ci commented Sep 29, 2021

Uh oh!

swift-ci commented Sep 29, 2021

Uh oh!

BradLarson left a comment

Choose a reason for hiding this comment

Uh oh!

rxwei commented Sep 29, 2021

Uh oh!

swift-ci commented Sep 29, 2021

Uh oh!

rxwei commented Sep 29, 2021

Uh oh!

swift-ci commented Sep 29, 2021

Uh oh!

swift-ci commented Sep 29, 2021

Uh oh!

rxwei commented Sep 30, 2021

Uh oh!

swift-ci commented Sep 30, 2021

Uh oh!

swift-ci commented Sep 30, 2021

Uh oh!

rxwei commented Sep 30, 2021

Uh oh!

rxwei commented Oct 1, 2021

Uh oh!

Uh oh!

rxwei commented Sep 29, 2021 •

edited

Loading