From 2915b6f44c295fb0bace586004bedf96f9173642 Mon Sep 17 00:00:00 2001 From: Dan Zheng Date: Sat, 21 Dec 2019 09:39:39 -0800 Subject: [PATCH 1/2] [AutoDiff] Diagnose unsupported coroutine differentiation. Coroutines are functions that can yield values and suspend/resume execution. SIL has dedicated coroutine types. Coroutines are applied via `begin_apply`. The `read` and `modify` accessors are coroutines. Coroutine differentiation requires extra support, so it should be diagnosed for now. Differentiation transform: - Generalize differentiation utilities from `ApplyInst` to `FullApplySite`. - `bool isArrayLiteralIntrinsic(FullApplySite)` - `void forEachApplyDirectResult(FullApplySite, llvm::function_ref<...>)` - `apply`: direct result is the `apply` result, unless the `apply` result has tuple-type and a single `destructure_tuple` user. Then, direct results are the `destructure_tuple` results. - `begin_apply`: direct results are the `begin_apply` results. - `try_apply`: direct results are successor blocks' arguments. - `bool LinearMapInfo::shouldDifferentiateApplySite(FullApplySite)` - Generalize activity analysis from `ApplyInst` to `FullApplySite`. - Propagate variedness for `FullApplySite` through direct/indirect results and `inout` arguments. - Propagate usefulness for `FullApplySite` through arguments. - Diagnose `begin_apply` instructions with active arguments and results in `PullbackEmitter::visitBeginApplyInst`. Sema: - Diagnose `@differentiable` attribute on `read` and `modify` coroutines. Resolves TF-1081. TF-1080 tracks coroutine differentiation support. TF-1083 tracks throwing function differentiation support. --- include/swift/AST/DiagnosticsSIL.def | 3 + include/swift/SIL/ApplySite.h | 13 ++ include/swift/SIL/SILInstruction.h | 48 +++---- .../Utils/Differentiation/Common.h | 25 ++-- .../Utils/Differentiation/LinearMapInfo.h | 2 +- .../Utils/Differentiation/PullbackEmitter.h | 2 + .../DifferentiableActivityAnalysis.cpp | 26 ++-- .../Utils/Differentiation/Common.cpp | 39 ++++-- .../Utils/Differentiation/JVPEmitter.cpp | 4 +- .../Utils/Differentiation/LinearMapInfo.cpp | 29 +++-- .../Utils/Differentiation/PullbackEmitter.cpp | 11 +- .../Utils/Differentiation/VJPEmitter.cpp | 2 +- lib/Sema/TypeCheckAttr.cpp | 8 +- test/AutoDiff/activity_analysis.swift | 119 ++++++++++++++++-- test/AutoDiff/control_flow_diagnostics.swift | 13 +- .../differentiable_attr_type_checking.swift | 22 ++++ ...ifferentiation_transform_diagnostics.swift | 35 ++++++ 17 files changed, 320 insertions(+), 81 deletions(-) diff --git a/include/swift/AST/DiagnosticsSIL.def b/include/swift/AST/DiagnosticsSIL.def index b81e0f6b618e1..f0210caec0edf 100644 --- a/include/swift/AST/DiagnosticsSIL.def +++ b/include/swift/AST/DiagnosticsSIL.def @@ -541,6 +541,9 @@ NOTE(autodiff_control_flow_not_supported,none, // TODO(TF-645): Remove when differentiation supports `ref_element_addr`. NOTE(autodiff_class_property_not_supported,none, "differentiating class properties is not yet supported", ()) +// TODO(TF-1080): Remove when differentiation supports `begin_apply`. +NOTE(autodiff_coroutines_not_supported,none, + "differentiation of coroutine calls is not yet supported", ()) NOTE(autodiff_missing_return,none, "missing return for differentiation", ()) diff --git a/include/swift/SIL/ApplySite.h b/include/swift/SIL/ApplySite.h index f98608cbfbeb8..cc98de2f40e45 100644 --- a/include/swift/SIL/ApplySite.h +++ b/include/swift/SIL/ApplySite.h @@ -503,6 +503,19 @@ class FullApplySite : public ApplySite { return getArguments().slice(getNumIndirectSILResults()); } + // SWIFT_ENABLE_TENSORFLOW + InoutArgumentRange getInoutArguments() const { + switch (getKind()) { + case FullApplySiteKind::ApplyInst: + return cast(getInstruction())->getInoutArguments(); + case FullApplySiteKind::TryApplyInst: + return cast(getInstruction())->getInoutArguments(); + case FullApplySiteKind::BeginApplyInst: + return cast(getInstruction())->getInoutArguments(); + } + } + // SWIFT_ENABLE_TENSORFLOW END + /// Returns true if \p op is the callee operand of this apply site /// and not an argument operand. bool isCalleeOperand(const Operand &op) const { diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h index 2cee2dc91ca7d..c32195c7f477b 100644 --- a/include/swift/SIL/SILInstruction.h +++ b/include/swift/SIL/SILInstruction.h @@ -2041,6 +2041,27 @@ class ApplyInstBase : public Base { /// does it have the given semantics? bool doesApplyCalleeHaveSemantics(SILValue callee, StringRef semantics); +// SWIFT_ENABLE_TENSORFLOW +/// Predicate used to filter InoutArgumentRange. +struct OperandToInoutArgument { + ArrayRef paramInfos; + OperandValueArrayRef arguments; + OperandToInoutArgument(ArrayRef paramInfos, + OperandValueArrayRef arguments) + : paramInfos(paramInfos), arguments(arguments) { + assert(paramInfos.size() == arguments.size()); + } + Optional operator()(unsigned long i) const { + if (paramInfos[i].isIndirectMutating()) + return arguments[i]; + return None; + } +}; + +using InoutArgumentRange = + OptionalTransformRange, OperandToInoutArgument>; +// SWIFT_ENABLE_TENSORFLOW END + /// The partial specialization of ApplyInstBase for full applications. /// Adds some methods relating to 'self' and to result types that don't /// make sense for partial applications. @@ -2147,31 +2168,14 @@ class ApplyInstBase } // SWIFT_ENABLE_TENSORFLOW -private: - /// Predicate used to filter InoutArgumentRange. - struct OperandToInoutArgument { - ArrayRef paramInfos; - OperandValueArrayRef arguments; - OperandToInoutArgument(const Impl &inst) - : paramInfos(inst.getSubstCalleeConv().getParameters()), - arguments(inst.getArgumentsWithoutIndirectResults()) { - assert(paramInfos.size() == arguments.size()); - } - Optional operator()(unsigned long i) const { - if (paramInfos[i].isIndirectMutating()) - return arguments[i]; - return None; - } - }; - -public: - using InoutArgumentRange = - OptionalTransformRange, OperandToInoutArgument>; /// Returns all `@inout` and `@inout_aliasable` arguments passed to the /// instruction. InoutArgumentRange getInoutArguments() const { - return InoutArgumentRange(indices(getArgumentsWithoutIndirectResults()), - OperandToInoutArgument(asImpl())); + auto &impl = asImpl(); + return InoutArgumentRange( + indices(getArgumentsWithoutIndirectResults()), + OperandToInoutArgument(impl.getSubstCalleeConv().getParameters(), + impl.getArgumentsWithoutIndirectResults())); } // SWIFT_ENABLE_TENSORFLOW END }; diff --git a/include/swift/SILOptimizer/Utils/Differentiation/Common.h b/include/swift/SILOptimizer/Utils/Differentiation/Common.h index d44b91e1452de..82c48b2541a35 100644 --- a/include/swift/SILOptimizer/Utils/Differentiation/Common.h +++ b/include/swift/SILOptimizer/Utils/Differentiation/Common.h @@ -50,9 +50,9 @@ namespace autodiff { /// This is being used to print short debug messages within the AD pass. raw_ostream &getADDebugStream(); -/// Returns true if this is an `ApplyInst` with `array.uninitialized_intrinsic` -/// semantics. -bool isArrayLiteralIntrinsic(ApplyInst *ai); +/// Returns true if this is an full apply site whose callee has +/// `array.uninitialized_intrinsic` semantics. +bool isArrayLiteralIntrinsic(FullApplySite applySite); /// If the given value `v` corresponds to an `ApplyInst` with /// `array.uninitialized_intrinsic` semantics, returns the corresponding @@ -76,11 +76,22 @@ ApplyInst *getAllocateUninitializedArrayIntrinsicElementAddress(SILValue v); /// tuple-typed and such a user exists. DestructureTupleInst *getSingleDestructureTupleUser(SILValue value); -/// Given an `apply` instruction, apply the given callback to each of its -/// direct results. If the `apply` instruction has a single `destructure_tuple` -/// user, apply the callback to the results of the `destructure_tuple` user. +/// Given a full apply site, apply the given callback to each of its +/// "direct results". +/// +/// - `apply` +/// Special case because `apply` returns a single (possibly tuple-typed) result +/// instead of multiple results. If the `apply` has a single +/// `destructure_tuple` user, treat the `destructure_tuple` results as the +/// `apply` direct results. +/// +/// - `begin_apply` +/// Apply callback to each `begin_apply` direct result. +/// +/// - `try_apply` +/// Apply callback to each `try_apply` successor basic block argument. void forEachApplyDirectResult( - ApplyInst *ai, llvm::function_ref resultCallback); + FullApplySite applySite, llvm::function_ref resultCallback); /// Given a function, gathers all of its formal results (both direct and /// indirect) in an order defined by its result type. Note that "formal results" diff --git a/include/swift/SILOptimizer/Utils/Differentiation/LinearMapInfo.h b/include/swift/SILOptimizer/Utils/Differentiation/LinearMapInfo.h index 5a4f0a4062cb8..f86053fb24c28 100644 --- a/include/swift/SILOptimizer/Utils/Differentiation/LinearMapInfo.h +++ b/include/swift/SILOptimizer/Utils/Differentiation/LinearMapInfo.h @@ -141,7 +141,7 @@ class LinearMapInfo { SILFunction *derivative); public: - bool shouldDifferentiateApplyInst(ApplyInst *ai); + bool shouldDifferentiateApplySite(FullApplySite applySite); bool shouldDifferentiateInstruction(SILInstruction *inst); LinearMapInfo(const LinearMapInfo &) = delete; diff --git a/include/swift/SILOptimizer/Utils/Differentiation/PullbackEmitter.h b/include/swift/SILOptimizer/Utils/Differentiation/PullbackEmitter.h index bce7fe721f6e5..8848c5d172321 100644 --- a/include/swift/SILOptimizer/Utils/Differentiation/PullbackEmitter.h +++ b/include/swift/SILOptimizer/Utils/Differentiation/PullbackEmitter.h @@ -338,6 +338,8 @@ class PullbackEmitter final : public SILInstructionVisitor { void visitApplyInst(ApplyInst *ai); + void visitBeginApplyInst(BeginApplyInst *bai); + /// Handle `struct` instruction. /// Original: y = struct (x0, x1, x2, ...) /// Adjoint: adj[x0] += struct_extract adj[y], #x0 diff --git a/lib/SILOptimizer/Analysis/DifferentiableActivityAnalysis.cpp b/lib/SILOptimizer/Analysis/DifferentiableActivityAnalysis.cpp index 71104fb98ea81..111bc741b2879 100644 --- a/lib/SILOptimizer/Analysis/DifferentiableActivityAnalysis.cpp +++ b/lib/SILOptimizer/Analysis/DifferentiableActivityAnalysis.cpp @@ -126,19 +126,21 @@ void DifferentiableActivityInfo::propagateVaried( // General rule: mark results as varied and recursively propagate variedness // to users of results. auto i = independentVariableIndex; - // Handle `apply`. - if (auto *ai = dyn_cast(inst)) { + // Handle full apply sites: `apply`, `try_apply`, and `begin_apply`. + if (FullApplySite::isa(inst)) { + FullApplySite applySite(inst); // If callee is non-varying, skip. - if (isWithoutDerivative(ai->getCallee())) + if (isWithoutDerivative(applySite.getCallee())) return; // If operand is varied, set all direct/indirect results and inout arguments // as varied. if (isVaried(operand->get(), i)) { - for (auto indRes : ai->getIndirectSILResults()) + for (auto indRes : applySite.getIndirectSILResults()) propagateVariedInwardsThroughProjections(indRes, i); - for (auto inoutArg : ai->getInoutArguments()) + for (auto inoutArg : applySite.getInoutArguments()) propagateVariedInwardsThroughProjections(inoutArg, i); - forEachApplyDirectResult(ai, [&](SILValue directResult) { + // Propagate variedness to apply site direct results. + forEachApplyDirectResult(applySite, [&](SILValue directResult) { setVariedAndPropagateToUsers(directResult, i); }); } @@ -218,7 +220,7 @@ void DifferentiableActivityInfo::propagateVariedInwardsThroughProjections( // Set value as varied and propagate to users. setVariedAndPropagateToUsers(value, independentVariableIndex); auto *inst = value->getDefiningInstruction(); - if (!inst || isa(inst)) + if (!inst || ApplySite::isa(inst)) return; // Standard propagation. for (auto &op : inst->getAllOperands()) @@ -262,11 +264,13 @@ void DifferentiableActivityInfo::propagateUseful( // Propagate usefulness for the given instruction: mark operands as useful and // recursively propagate usefulness to defining instructions of operands. auto i = dependentVariableIndex; - // Handle indirect results in `apply`. - if (auto *ai = dyn_cast(inst)) { - if (isWithoutDerivative(ai->getCallee())) + // Handle full apply sites: `apply`, `try_apply`, and `begin_apply`. + if (FullApplySite::isa(inst)) { + FullApplySite applySite(inst); + // If callee is non-varying, skip. + if (isWithoutDerivative(applySite.getCallee())) return; - for (auto arg : ai->getArgumentsWithoutIndirectResults()) + for (auto arg : applySite.getArgumentsWithoutIndirectResults()) setUsefulAndPropagateToOperands(arg, i); } // Handle store-like instructions: diff --git a/lib/SILOptimizer/Utils/Differentiation/Common.cpp b/lib/SILOptimizer/Utils/Differentiation/Common.cpp index a5dab74e739c0..8ba7f145bdd3c 100644 --- a/lib/SILOptimizer/Utils/Differentiation/Common.cpp +++ b/lib/SILOptimizer/Utils/Differentiation/Common.cpp @@ -26,8 +26,9 @@ namespace autodiff { raw_ostream &getADDebugStream() { return llvm::dbgs() << "[AD] "; } -bool isArrayLiteralIntrinsic(ApplyInst *ai) { - return ai->hasSemantics("array.uninitialized_intrinsic"); +bool isArrayLiteralIntrinsic(FullApplySite applySite) { + return doesApplyCalleeHaveSemantics(applySite.getCalleeOrigin(), + "array.uninitialized_intrinsic"); } ApplyInst *getAllocateUninitializedArrayIntrinsic(SILValue v) { @@ -71,14 +72,34 @@ DestructureTupleInst *getSingleDestructureTupleUser(SILValue value) { } void forEachApplyDirectResult( - ApplyInst *ai, llvm::function_ref resultCallback) { - if (!ai->getType().is()) { - resultCallback(ai); - return; + FullApplySite applySite, + llvm::function_ref resultCallback) { + switch (applySite.getKind()) { + case FullApplySiteKind::ApplyInst: { + auto *ai = cast(applySite.getInstruction()); + if (!ai->getType().is()) { + resultCallback(ai); + return; + } + if (auto *dti = getSingleDestructureTupleUser(ai)) + for (auto directtResult : dti->getResults()) + resultCallback(directtResult); + break; + } + case FullApplySiteKind::BeginApplyInst: { + auto *bai = cast(applySite.getInstruction()); + for (auto directResult : bai->getResults()) + resultCallback(directResult); + break; + } + case FullApplySiteKind::TryApplyInst: { + auto *tai = cast(applySite.getInstruction()); + for (auto *succBB : tai->getSuccessorBlocks()) + for (auto *arg : succBB->getArguments()) + resultCallback(arg); + break; + } } - if (auto *dti = getSingleDestructureTupleUser(ai)) - for (auto result : dti->getResults()) - resultCallback(result); } void collectAllFormalResultsInTypeOrder(SILFunction &function, diff --git a/lib/SILOptimizer/Utils/Differentiation/JVPEmitter.cpp b/lib/SILOptimizer/Utils/Differentiation/JVPEmitter.cpp index ec1618a4260d8..1743df6d070d0 100644 --- a/lib/SILOptimizer/Utils/Differentiation/JVPEmitter.cpp +++ b/lib/SILOptimizer/Utils/Differentiation/JVPEmitter.cpp @@ -746,7 +746,7 @@ CLONE_AND_EMIT_TANGENT(DestructureTuple, dti) { void JVPEmitter::emitTangentForApplyInst( ApplyInst *ai, SILAutoDiffIndices actualIndices, CanSILFunctionType originalDifferentialType) { - assert(differentialInfo.shouldDifferentiateApplyInst(ai)); + assert(differentialInfo.shouldDifferentiateApplySite(ai)); auto *bb = ai->getParent(); auto loc = ai->getLoc(); auto &diffBuilder = getDifferentialBuilder(); @@ -1184,7 +1184,7 @@ void JVPEmitter::visitInstructionsInBlock(SILBasicBlock *bb) { void JVPEmitter::visitApplyInst(ApplyInst *ai) { // If the function should not be differentiated or its the array literal // initialization intrinsic, just do standard cloning. - if (!differentialInfo.shouldDifferentiateApplyInst(ai) || + if (!differentialInfo.shouldDifferentiateApplySite(ai) || isArrayLiteralIntrinsic(ai)) { LLVM_DEBUG(getADDebugStream() << "No active results:\n" << *ai << '\n'); TypeSubstCloner::visitApplyInst(ai); diff --git a/lib/SILOptimizer/Utils/Differentiation/LinearMapInfo.cpp b/lib/SILOptimizer/Utils/Differentiation/LinearMapInfo.cpp index 16c4bf613e6f0..13a36cf2318e4 100644 --- a/lib/SILOptimizer/Utils/Differentiation/LinearMapInfo.cpp +++ b/lib/SILOptimizer/Utils/Differentiation/LinearMapInfo.cpp @@ -411,7 +411,7 @@ void LinearMapInfo::generateDifferentiationDataStructures( // Add linear map field to struct for active `apply` instructions. // Skip array literal intrinsic applications since array literal // initialization is linear and handled separately. - if (!shouldDifferentiateApplyInst(ai) || isArrayLiteralIntrinsic(ai)) + if (!shouldDifferentiateApplySite(ai) || isArrayLiteralIntrinsic(ai)) continue; LLVM_DEBUG(getADDebugStream() << "Adding linear map struct field for " @@ -454,26 +454,29 @@ void LinearMapInfo::generateDifferentiationDataStructures( /// there is a `store` of an active value into the array's buffer. /// 3. The instruction has both an active result (direct or indirect) and an /// active argument. -bool LinearMapInfo::shouldDifferentiateApplyInst(ApplyInst *ai) { +bool LinearMapInfo::shouldDifferentiateApplySite(FullApplySite applySite) { // Function applications with an inout argument should be differentiated. - for (auto inoutArg : ai->getInoutArguments()) + for (auto inoutArg : applySite.getInoutArguments()) if (activityInfo.isActive(inoutArg, indices)) return true; bool hasActiveDirectResults = false; - forEachApplyDirectResult(ai, [&](SILValue directResult) { + forEachApplyDirectResult(applySite, [&](SILValue directResult) { hasActiveDirectResults |= activityInfo.isActive(directResult, indices); }); - bool hasActiveIndirectResults = llvm::any_of(ai->getIndirectSILResults(), - [&](SILValue result) { return activityInfo.isActive(result, indices); }); + bool hasActiveIndirectResults = + llvm::any_of(applySite.getIndirectSILResults(), [&](SILValue result) { + return activityInfo.isActive(result, indices); + }); bool hasActiveResults = hasActiveDirectResults || hasActiveIndirectResults; // TODO: Pattern match to make sure there is at least one `store` to the // array's active buffer. - if (isArrayLiteralIntrinsic(ai) && hasActiveResults) + // if (isArrayLiteralIntrinsic(ai) && hasActiveResults) + if (isArrayLiteralIntrinsic(applySite) && hasActiveResults) return true; - auto arguments = ai->getArgumentsWithoutIndirectResults(); + auto arguments = applySite.getArgumentsWithoutIndirectResults(); bool hasActiveArguments = llvm::any_of(arguments, [&](SILValue arg) { return activityInfo.isActive(arg, indices); }); return hasActiveResults && hasActiveArguments; @@ -483,8 +486,8 @@ bool LinearMapInfo::shouldDifferentiateApplyInst(ApplyInst *ai) { /// given the differentiation indices of the instruction's parent function. /// Whether the instruction should be differentiated is determined sequentially /// from any of the following conditions: -/// 1. The instruction is an `apply` and `shouldDifferentiateApplyInst` returns -/// true. +/// 1. The instruction is a full apply site and `shouldDifferentiateApplyInst` +/// returns true. /// 2. The instruction has a source operand and a destination operand, both /// being active. /// 3. The instruction is an allocation instruction and has an active result. @@ -492,10 +495,10 @@ bool LinearMapInfo::shouldDifferentiateApplyInst(ApplyInst *ai) { /// ending, or destroying on an active operand. /// 5. The instruction creates an SSA copy of an active operand. bool LinearMapInfo::shouldDifferentiateInstruction(SILInstruction *inst) { - // An `apply` with an active argument and an active result (direct or + // A full apply site with an active argument and an active result (direct or // indirect) should be differentiated. - if (auto *ai = dyn_cast(inst)) - return shouldDifferentiateApplyInst(ai); + if (FullApplySite::isa(inst)) + return shouldDifferentiateApplySite(FullApplySite(inst)); // Anything with an active result and an active operand should be // differentiated. auto hasActiveOperands = llvm::any_of(inst->getAllOperands(), diff --git a/lib/SILOptimizer/Utils/Differentiation/PullbackEmitter.cpp b/lib/SILOptimizer/Utils/Differentiation/PullbackEmitter.cpp index 32090416da4fc..afc5a41a6b548 100644 --- a/lib/SILOptimizer/Utils/Differentiation/PullbackEmitter.cpp +++ b/lib/SILOptimizer/Utils/Differentiation/PullbackEmitter.cpp @@ -1078,7 +1078,7 @@ PullbackEmitter::getArrayAdjointElementBuffer(SILValue arrayAdjoint, } void PullbackEmitter::visitApplyInst(ApplyInst *ai) { - assert(getPullbackInfo().shouldDifferentiateApplyInst(ai)); + assert(getPullbackInfo().shouldDifferentiateApplySite(ai)); // Skip `array.uninitialized_intrinsic` intrinsic applications, which have // special `store` and `copy_addr` support. if (isArrayLiteralIntrinsic(ai)) @@ -1273,6 +1273,15 @@ void PullbackEmitter::visitStructInst(StructInst *si) { } } +void PullbackEmitter::visitBeginApplyInst(BeginApplyInst *bai) { + // Diagnose `begin_apply` instructions. + // Coroutine differentiation is not yet supported. + getContext().emitNondifferentiabilityError( + bai, getInvoker(), diag::autodiff_coroutines_not_supported); + errorOccurred = true; + return; +} + void PullbackEmitter::visitStructExtractInst(StructExtractInst *sei) { assert(!sei->getField()->getAttrs().hasAttribute() && "`struct_extract` with `@noDerivative` field should not be " diff --git a/lib/SILOptimizer/Utils/Differentiation/VJPEmitter.cpp b/lib/SILOptimizer/Utils/Differentiation/VJPEmitter.cpp index 8cc93bed6912c..ad6e31f12f3ac 100644 --- a/lib/SILOptimizer/Utils/Differentiation/VJPEmitter.cpp +++ b/lib/SILOptimizer/Utils/Differentiation/VJPEmitter.cpp @@ -430,7 +430,7 @@ void VJPEmitter::visitSwitchEnumInst(SwitchEnumInst *sei) { void VJPEmitter::visitApplyInst(ApplyInst *ai) { // If the function should not be differentiated or its the array literal // initialization intrinsic, just do standard cloning. - if (!pullbackInfo.shouldDifferentiateApplyInst(ai) || + if (!pullbackInfo.shouldDifferentiateApplySite(ai) || isArrayLiteralIntrinsic(ai)) { LLVM_DEBUG(getADDebugStream() << "No active results:\n" << *ai << '\n'); TypeSubstCloner::visitApplyInst(ai); diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index a84a327092a5f..5d2ba05a4f6e5 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -3857,10 +3857,12 @@ DifferentiableAttributeParameterIndicesRequest::evaluate( original = nullptr; } } - // Setters are not yet supported. - // TODO(TF-129): Remove this when differentiation supports inout parameters. + // Non-`get` accessors are not yet supported: `set`, `read`, and `modify`. + // TODO(TF-129): Enable `set` when differentiation supports inout parameters. + // TODO(TF-1080): Enable `read` and `modify` when differentiation supports + // coroutines. if (auto *accessor = dyn_cast_or_null(original)) - if (accessor->isSetter()) + if (!accessor->isGetter()) original = nullptr; // Global immutable vars, for example, have no getter, and therefore trigger diff --git a/test/AutoDiff/activity_analysis.swift b/test/AutoDiff/activity_analysis.swift index f4b6933606f4c..c841599af4fa4 100644 --- a/test/AutoDiff/activity_analysis.swift +++ b/test/AutoDiff/activity_analysis.swift @@ -413,13 +413,112 @@ func activeInoutArgNonactiveInitialResult(_ x: Float) -> Float { } // CHECK-LABEL: [AD] Activity info for ${{.*}}activeInoutArgNonactiveInitialResult{{.*}} at (source=0 parameters=(0)) -// CHECK-LABEL: [ACTIVE] %0 = argument of bb0 : $Float -// CHECK-LABEL: [ACTIVE] %2 = alloc_stack $Float, var, name "result" -// CHECK-LABEL: [NONE] // function_ref Float.init(_builtinIntegerLiteral:) -// CHECK-LABEL: [USEFUL] %6 = apply %5(%3, %4) : $@convention(method) (Builtin.IntLiteral, @thin Float.Type) -> Float -// CHECK-LABEL: [USEFUL] %8 = metatype $@thin Float.Type -// CHECK-LABEL: [ACTIVE] %9 = begin_access [modify] [static] %2 : $*Float -// CHECK-LABEL: [NONE] // function_ref static Float.+= infix(_:_:) -// CHECK-LABEL: [NONE] %11 = apply %10(%9, %0, %8) : $@convention(method) (@inout Float, Float, @thin Float.Type) -> () -// CHECK-LABEL: [ACTIVE] %13 = begin_access [read] [static] %2 : $*Float -// CHECK-LABEL: [ACTIVE] %14 = load [trivial] %13 : $*Float +// CHECK: [ACTIVE] %0 = argument of bb0 : $Float +// CHECK: [ACTIVE] %2 = alloc_stack $Float, var, name "result" +// CHECK: [NONE] // function_ref Float.init(_builtinIntegerLiteral:) +// CHECK: [USEFUL] %6 = apply %5(%3, %4) : $@convention(method) (Builtin.IntLiteral, @thin Float.Type) -> Float +// CHECK: [USEFUL] %8 = metatype $@thin Float.Type +// CHECK: [ACTIVE] %9 = begin_access [modify] [static] %2 : $*Float +// CHECK: [NONE] // function_ref static Float.+= infix(_:_:) +// CHECK: [NONE] %11 = apply %10(%9, %0, %8) : $@convention(method) (@inout Float, Float, @thin Float.Type) -> () +// CHECK: [ACTIVE] %13 = begin_access [read] [static] %2 : $*Float +// CHECK: [ACTIVE] %14 = load [trivial] %13 : $*Float + +//===----------------------------------------------------------------------===// +// Throwing function differentiation (`try_apply`) +//===----------------------------------------------------------------------===// + +// TF-433: Test `try_apply`. + +func rethrowing(_ x: () throws -> Void) rethrows -> Void {} + +// expected-error @+1 {{function is not differentiable}} +@differentiable +// expected-note @+1 {{when differentiating this function definition}} +func testTryApply(_ x: Float) -> Float { + // expected-note @+1 {{cannot differentiate unsupported control flow}} + rethrowing({}) + return x +} + +// TF-433: differentiation diagnoses `try_apply` before activity info is printed. +// CHECK-NOT: [AD] Activity info for ${{.*}}testTryApply{{.*}} at (source=0 parameters=(0)) + +//===----------------------------------------------------------------------===// +// Coroutine differentiation (`begin_apply`) +//===----------------------------------------------------------------------===// + +struct HasCoroutineAccessors: Differentiable { + var stored: Float + var computed: Float { + // `_read` is a coroutine: `(Self) -> () -> ()`. + _read { yield stored } + // `_modify` is a coroutine: `(inout Self) -> () -> ()`. + _modify { yield &stored } + } +} +// expected-error @+1 {{function is not differentiable}} +@differentiable +// expected-note @+1 {{when differentiating this function definition}} +func testAccessorCoroutines(_ x: HasCoroutineAccessors) -> HasCoroutineAccessors { + var x = x + // expected-note @+1 {{differentiation of coroutine calls is not yet supported}} + x.computed = x.computed + return x +} + +// CHECK-LABEL: [AD] Activity info for ${{.*}}testAccessorCoroutines{{.*}} at (source=0 parameters=(0)) +// CHECK: [ACTIVE] %0 = argument of bb0 : $HasCoroutineAccessors +// CHECK: [ACTIVE] %2 = alloc_stack $HasCoroutineAccessors, var, name "x" +// CHECK: [ACTIVE] %4 = begin_access [read] [static] %2 : $*HasCoroutineAccessors +// CHECK: [VARIED] %5 = load [trivial] %4 : $*HasCoroutineAccessors +// CHECK: [NONE] // function_ref HasCoroutineAccessors.computed.read +// CHECK: [VARIED] (**%7**, %8) = begin_apply %6(%5) : $@yield_once @convention(method) (HasCoroutineAccessors) -> @yields Float +// CHECK: [VARIED] (%7, **%8**) = begin_apply %6(%5) : $@yield_once @convention(method) (HasCoroutineAccessors) -> @yields Float +// CHECK: [VARIED] %9 = alloc_stack $Float +// CHECK: [VARIED] %11 = load [trivial] %9 : $*Float +// CHECK: [ACTIVE] %14 = begin_access [modify] [static] %2 : $*HasCoroutineAccessors +// CHECK: [NONE] // function_ref HasCoroutineAccessors.computed.modify +// CHECK: %15 = function_ref @$s17activity_analysis21HasCoroutineAccessorsV8computedSfvM : $@yield_once @convention(method) (@inout HasCoroutineAccessors) -> @yields @inout Float +// CHECK: [VARIED] (**%16**, %17) = begin_apply %15(%14) : $@yield_once @convention(method) (@inout HasCoroutineAccessors) -> @yields @inout Float +// CHECK: [VARIED] (%16, **%17**) = begin_apply %15(%14) : $@yield_once @convention(method) (@inout HasCoroutineAccessors) -> @yields @inout Float +// CHECK: [ACTIVE] %22 = begin_access [read] [static] %2 : $*HasCoroutineAccessors +// CHECK: [ACTIVE] %23 = load [trivial] %22 : $*HasCoroutineAccessors + +// TF-1078: Test `begin_apply` active `inout` argument. +// `Array.subscript.modify` is the applied coroutine. + +// expected-error @+1 {{function is not differentiable}} +@differentiable +// expected-note @+1 {{when differentiating this function definition}} +func testBeginApplyActiveInoutArgument(array: [Float], x: Float) -> Float { + var array = array + // Array subscript assignment below calls `Array.subscript.modify`. + // expected-note @+1 {{differentiation of coroutine calls is not yet supported}} + array[0] = x + return array[0] +} + +// CHECK-LABEL: [AD] Activity info for ${{.*}}testBeginApplyActiveInoutArgument{{.*}} at (source=0 parameters=(0 1)) +// CHECK: [ACTIVE] %0 = argument of bb0 : $Array +// CHECK: [VARIED] %1 = argument of bb0 : $Float +// CHECK: [ACTIVE] %4 = alloc_stack $Array, var, name "array" +// CHECK: [ACTIVE] %5 = copy_value %0 : $Array +// CHECK: [USEFUL] %7 = integer_literal $Builtin.IntLiteral, 0 +// CHECK: [USEFUL] %8 = metatype $@thin Int.Type +// CHECK: [NONE] // function_ref Int.init(_builtinIntegerLiteral:) +// CHECK: [USEFUL] %10 = apply %9(%7, %8) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int +// CHECK: [ACTIVE] %11 = begin_access [modify] [static] %4 : $*Array +// CHECK: [NONE] // function_ref Array.subscript.modify +// CHECK: [VARIED] (**%13**, %14) = begin_apply %12(%10, %11) : $@yield_once @convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> @yields @inout τ_0_0 +// CHECK: [VARIED] (%13, **%14**) = begin_apply %12(%10, %11) : $@yield_once @convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> @yields @inout τ_0_0 +// CHECK: [USEFUL] %18 = integer_literal $Builtin.IntLiteral, 0 +// CHECK: [USEFUL] %19 = metatype $@thin Int.Type +// CHECK: [NONE] // function_ref Int.init(_builtinIntegerLiteral:) +// CHECK: [USEFUL] %21 = apply %20(%18, %19) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int +// CHECK: [ACTIVE] %22 = begin_access [read] [static] %4 : $*Array +// CHECK: [ACTIVE] %23 = load_borrow %22 : $*Array +// CHECK: [ACTIVE] %24 = alloc_stack $Float +// CHECK: [NONE] // function_ref Array.subscript.getter +// CHECK: [NONE] %26 = apply %25(%24, %21, %23) : $@convention(method) <τ_0_0> (Int, @guaranteed Array<τ_0_0>) -> @out τ_0_0 +// CHECK: [ACTIVE] %27 = load [trivial] %24 : $*Float diff --git a/test/AutoDiff/control_flow_diagnostics.swift b/test/AutoDiff/control_flow_diagnostics.swift index 2b17d7c77b72f..ded6ab27c950e 100644 --- a/test/AutoDiff/control_flow_diagnostics.swift +++ b/test/AutoDiff/control_flow_diagnostics.swift @@ -72,7 +72,18 @@ func nested_loop(_ x: Float) -> Float { return outer } -// Test `try_apply`. +// TF-433: Test throwing functions. + +func rethrowing(_ x: () throws -> Void) rethrows -> Void {} + +// expected-error @+1 {{function is not differentiable}} +@differentiable +// expected-note @+1 {{when differentiating this function definition}} +func testTryApply(_ x: Float) -> Float { + // expected-note @+1 {{cannot differentiate unsupported control flow}} + rethrowing({}) + return x +} // expected-error @+1 {{function is not differentiable}} @differentiable diff --git a/test/AutoDiff/differentiable_attr_type_checking.swift b/test/AutoDiff/differentiable_attr_type_checking.swift index e76e9c566b95a..04d64d0463a1d 100644 --- a/test/AutoDiff/differentiable_attr_type_checking.swift +++ b/test/AutoDiff/differentiable_attr_type_checking.swift @@ -1053,3 +1053,25 @@ class Sub : Super { @differentiable(wrt: x, vjp: vjp) override func testSuperclassDerivatives(_ x: Float) -> Float { x } } + +// Test unsupported accessors: `set`, `_read`, `_modify`. + +struct UnsupportedAccessors: Differentiable { + var stored: Float + var computed: Float { + // `set` has an `inout` parameter: `(inout Self) -> (Float) -> ()`. + // expected-error @+1 {{'@differentiable' attribute cannot be applied to this declaration}} + @differentiable + set { stored = newValue } + + // `_read` is a coroutine: `(Self) -> () -> ()`. + // expected-error @+1 {{'@differentiable' attribute cannot be applied to this declaration}} + @differentiable + _read { yield stored } + + // `_modify` is a coroutine: `(inout Self) -> () -> ()`. + // expected-error @+1 {{'@differentiable' attribute cannot be applied to this declaration}} + @differentiable + _modify { yield &stored } + } +} diff --git a/test/AutoDiff/differentiation_transform_diagnostics.swift b/test/AutoDiff/differentiation_transform_diagnostics.swift index 66d15f3c93940..d6fe506829d56 100644 --- a/test/AutoDiff/differentiation_transform_diagnostics.swift +++ b/test/AutoDiff/differentiation_transform_diagnostics.swift @@ -460,6 +460,41 @@ let _: @differentiable (Float) -> Float = TF_675().method // expected-note @+1 {{cannot convert a direct method reference to a '@differentiable' function; use an explicit closure instead}} _ = gradient(at: Float(1), Float(2), in: (+) as @differentiable (Float, @noDerivative Float) -> Float) +//===----------------------------------------------------------------------===// +// Coroutines (SIL function yields, `begin_apply`) (not yet supported) +//===----------------------------------------------------------------------===// + +struct HasCoroutineAccessors: Differentiable { + var stored: Float + var computed: Float { + // `_read` is a coroutine: `(Self) -> () -> ()`. + _read { yield stored } + // `_modify` is a coroutine: `(inout Self) -> () -> ()`. + _modify { yield &stored } + } +} +// expected-error @+1 {{function is not differentiable}} +@differentiable +// expected-note @+1 {{when differentiating this function definition}} +func testAccessorCoroutines(_ x: HasCoroutineAccessors) -> HasCoroutineAccessors { + var x = x + // expected-note @+1 {{differentiation of coroutine calls is not yet supported}} + x.computed = x.computed + return x +} + +// TF-1078: `Array.subscript.modify` is a coroutine. +// expected-error @+1 {{function is not differentiable}} +@differentiable +// expected-note @+1 {{when differentiating this function definition}} +func TF_1078(array: [Float], x: Float) -> Float { + var array = array + // Array subscript assignment below calls `Array.subscript.modify`. + // expected-note @+1 {{differentiation of coroutine calls is not yet supported}} + array[0] = x + return array[0] +} + //===----------------------------------------------------------------------===// // Conversion to `@differentiable(linear)` (not yet supported) //===----------------------------------------------------------------------===// From 7897243a683b4d83d23deb16e308275b80a102e8 Mon Sep 17 00:00:00 2001 From: Dan Zheng Date: Sat, 21 Dec 2019 11:56:30 -0800 Subject: [PATCH 2/2] Fix typo. --- lib/SILOptimizer/Utils/Differentiation/Common.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/SILOptimizer/Utils/Differentiation/Common.cpp b/lib/SILOptimizer/Utils/Differentiation/Common.cpp index 8ba7f145bdd3c..51ae7936c606a 100644 --- a/lib/SILOptimizer/Utils/Differentiation/Common.cpp +++ b/lib/SILOptimizer/Utils/Differentiation/Common.cpp @@ -82,8 +82,8 @@ void forEachApplyDirectResult( return; } if (auto *dti = getSingleDestructureTupleUser(ai)) - for (auto directtResult : dti->getResults()) - resultCallback(directtResult); + for (auto directResult : dti->getResults()) + resultCallback(directResult); break; } case FullApplySiteKind::BeginApplyInst: {