-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[AutoDiff] Fix variedness propagation for apply inout arguments.
#28352
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
[AutoDiff] Fix variedness propagation for apply inout arguments.
#28352
Conversation
Propagate variedness from `apply` argument operands to `apply` inout arguments (representing results). `apply` inout arguments are now correctly marked as active, triggering non-differentiability errors. Add `ApplyInstBase::getInoutArguments` for iterating over `@inout` and `@inout_aliasable` arguments. Add non-differentiability diagnostics and activity info tests. Resolves TF-974.
| OptionalTransformRange<IntRange<unsigned long>, OperandToInoutArgument>; | ||
| /// Returns all `@inout` and `@inout_aliasable` arguments passed to the | ||
| /// instruction. | ||
| InoutArgumentRange getInoutArguments() const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: getInoutArguments is currently defined in the "full application" case of ApplyInstBase. This required duplicating asImpl from the "partial application" ApplyInstBase superclass, which is a private function.
Other options:
- Define
getInoutArgumentsas a top-level function inDifferentiation.cppspecialized for justApplyInst. No problems there. - Move
getInoutArgumentsto the "partial application" superclassApplyInstBase. This requires more duplication becausegetArgumentsWithoutIndirectResultsis defined only in the "full application"ApplyInstBase.
include/swift/SIL/SILInstruction.h
Outdated
| Optional<SILValue> operator()(unsigned long i) const { | ||
| auto paramInfo = inst.getSubstCalleeConv().getParameters()[i]; | ||
| auto argument = inst.getArgumentsWithoutIndirectResults()[i]; | ||
| if (paramInfo.isIndirectMutating()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: @inout-ness is represented in SILParameterInfo. It's quite more complicated to get SILParameterInfo from just an argument Operand (need to use operand index to compute argument index) so I decided to iterate over argument indices.
|
@swift-ci Please test tensorflow |
`Array.append` cannot be differentiated because differentiation does not yet support function applications with `inout` arguments. These latent bugs were uncovered in swiftlang/swift#28352, which correctly diagnoses function applications with `inout` arguments. https://bugs.swift.org/browse/TF-129 tracks `inout` argument support. In the meantime, rewrite `Array.append` using `Array.+`.
`mutating` functions like `Array.append` and `ActionValueCalculator.loss` cannot be differentiated because differentiation does not yet support function applications with `inout` arguments. These latent bugs were uncovered in swiftlang/swift#28352, which correctly diagnoses function applications with `inout` arguments. https://bugs.swift.org/browse/TF-129 tracks `inout` argument support. In the meantime, disable affected code and tests.
Propagate variedness from
applyargument operands toapplyinout arguments(representing results).
applyinout arguments are now correctly marked asactive, triggering non-differentiability errors.
Add
ApplyInstBase::getInoutArgumentsfor iterating over@inoutand@inout_aliasablearguments.Add non-differentiability diagnostics and activity info tests.
Resolves TF-974.
Example: