Skip to content

Conversation

integralpro
Copy link
Contributor

@integralpro integralpro commented Apr 23, 2021

The auto-differentiation mechanism in Swift creates a number of function reabstraction thunks. Most of them already use OSSA form but there is a couple of places that needs to be enabled.

The change enables SSA ownership for:

  • lib/SILOptimizer/Differentiation/Thunk.cpp:getOrCreateSubsetParametersThunkForLinearMap
  • lib/SILOptimizer/Mandatory/Differentiation.cpp:promoteCurryThunkApplicationToDifferentiableFunction()

Resolves [SR-13929]: Enable ownership for all Differentiation-generated thunks.

@integralpro
Copy link
Contributor Author

@swift-ci Please smoke test

@integralpro
Copy link
Contributor Author

@swift-ci please smoke test linux

@integralpro
Copy link
Contributor Author

@swift-ci Please test

@integralpro integralpro marked this pull request as ready for review April 26, 2021 14:41
@@ -121,7 +121,7 @@ getOrCreateSubsetParametersThunkForLinearMap(
SILOptFunctionBuilder &fb, SILFunction *assocFn,
CanSILFunctionType origFnType, CanSILFunctionType linearMapType,
CanSILFunctionType targetType, AutoDiffDerivativeFunctionKind kind,
AutoDiffConfig desiredConfig, AutoDiffConfig actualConfig,
const AutoDiffConfig &desiredConfig, const AutoDiffConfig &actualConfig,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: AutoDiffConfig is a 3-word trivial struct and we are currently passing it by value everywhere. For consistency it might be better to keep it as is and, if passing by reference is desirable, change it everywhere in a separate patch.

Copy link
Contributor

Choose a reason for hiding this comment

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

... but given that all tests have passed, feel free to disregard this and merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants