From 7930bca16f969ddbfbb3da007b32381a07ee734a Mon Sep 17 00:00:00 2001 From: Dan Zheng Date: Wed, 28 Aug 2019 20:56:59 -0700 Subject: [PATCH] [AutoDiff] Temporarily disable non-varied result warning. Previously, a warning and fixit was generated for differentiable functions with non-varied results. ``` tf-775.swift:3:3: warning: result does not depend on differentiation arguments and will always have a zero derivative; do you want to use 'withoutDerivative(at:)'? .zero ^ withoutDerivative(at: ) ``` However, TF-775 exposes that the fixit does not work and the warning is unsilenceable. This patch temporarily disables the warning, as a robust fix requires non-trivial activity analysis changes. A lack of warning is better than an unsilenceable false positive. TF-788 tracks re-enabling the warning. Add regression test to ensure that unsilenceable warnings will not happen again. --- lib/SILOptimizer/Mandatory/Differentiation.cpp | 15 ++++++++++----- test/AutoDiff/autodiff_diagnostics.swift | 18 +++++++++++++++--- test/AutoDiff/forward_mode_diagnostics.swift | 3 ++- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/lib/SILOptimizer/Mandatory/Differentiation.cpp b/lib/SILOptimizer/Mandatory/Differentiation.cpp index 23ef6f2b2887d..54af86daf1374 100644 --- a/lib/SILOptimizer/Mandatory/Differentiation.cpp +++ b/lib/SILOptimizer/Mandatory/Differentiation.cpp @@ -4594,12 +4594,13 @@ class JVPEmitter final attr->getIndices().parameters->getNumIndices()); auto origParamArgs = original->getArgumentsWithoutIndirectResults(); - // Check if result is not varied. + // TODO(TF-788): Re-enable non-varied result warning. + /* + // Emit a warning and fixit if original result is not varied, because it + // will always have a zero derivative. SmallVector origFormalResults; collectAllFormalResultsInTypeOrder(*original, origFormalResults); auto origResult = origFormalResults[getIndices().source]; - // Emit warning if original result is not varied, because it will always - // have a zero derivative. if (!activityInfo.isVaried(origResult, getIndices().parameters)) { // Emit fixit if original result has a valid source location. auto startLoc = origResult.getLoc().getStartSourceLoc(); @@ -4610,6 +4611,7 @@ class JVPEmitter final .fixItInsertAfter(endLoc, ")"); } } + */ auto *diffEntry = getDifferential().getEntryBlock(); diffBuilder.setInsertionPoint( @@ -5624,8 +5626,10 @@ class PullbackEmitter final : public SILInstructionVisitor { SmallVector origFormalResults; collectAllFormalResultsInTypeOrder(original, origFormalResults); auto origResult = origFormalResults[getIndices().source]; - // Emit warning if original result is not varied, because it will always - // have a zero derivative. + // TODO(TF-788): Re-enable non-varied result warning. + /* + // Emit a warning and fixit if original result is not varied, because it + // will always have a zero derivative. if (!getActivityInfo().isVaried(origResult, getIndices().parameters)) { // Emit fixit if original result has a valid source location. auto startLoc = origResult.getLoc().getStartSourceLoc(); @@ -5636,6 +5640,7 @@ class PullbackEmitter final : public SILInstructionVisitor { .fixItInsertAfter(endLoc, ")"); } } + */ builder.setInsertionPoint( pullbackEntry, getNextFunctionLocalAllocationInsertionPoint()); if (seed->getType().isAddress()) { diff --git a/test/AutoDiff/autodiff_diagnostics.swift b/test/AutoDiff/autodiff_diagnostics.swift index dbdddd681ff81..d0fdd2fb7b4c2 100644 --- a/test/AutoDiff/autodiff_diagnostics.swift +++ b/test/AutoDiff/autodiff_diagnostics.swift @@ -50,11 +50,13 @@ _ = gradient(at: NoDerivativeProperty(x: 1, y: 1)) { s -> Float in return tmp.x } _ = gradient(at: NoDerivativeProperty(x: 1, y: 1)) { s in - // expected-warning @+1 {{result does not depend on differentiation arguments and will always have a zero derivative; do you want to use 'withoutDerivative(at:)'?}} {{10-10=withoutDerivative(at:}} {{13-13=)}} + // TODO(TF-788): Re-enable non-varied result warning. + // xpected-warning @+1 {{result does not depend on differentiation arguments and will always have a zero derivative; do you want to use 'withoutDerivative(at:)'?}} {{10-10=withoutDerivative(at:}} {{13-13=)}} return s.y } _ = gradient(at: NoDerivativeProperty(x: 1, y: 1)) { - // expected-warning @+1 {{result does not depend on differentiation arguments and will always have a zero derivative; do you want to use 'withoutDerivative(at:)'?}} {{3-3=withoutDerivative(at:}} {{7-7=)}} + // TODO(TF-788): Re-enable non-varied result warning. + // xpected-warning @+1 {{result does not depend on differentiation arguments and will always have a zero derivative; do you want to use 'withoutDerivative(at:)'?}} {{3-3=withoutDerivative(at:}} {{7-7=)}} $0.y } @@ -295,10 +297,20 @@ func one() -> Float { } @differentiable func nonVariedResult(_ x: Float) -> Float { - // expected-warning @+1 {{result does not depend on differentiation arguments and will always have a zero derivative; do you want to use 'withoutDerivative(at:)'?}} {{10-10=withoutDerivative(at:}} {{15-15=)}} + // TODO(TF-788): Re-enable non-varied result warning. + // xpected-warning @+1 {{result does not depend on differentiation arguments and will always have a zero derivative; do you want to use 'withoutDerivative(at:)'?}} {{10-10=withoutDerivative(at:}} {{15-15=)}} return one() } +// Check that `withoutDerivative(at:)` silences the warning. + +struct TF_775: Differentiable { + @differentiable(wrt: (self)) + func nonVariedResult(_ input: Float) -> Float { + withoutDerivative(at: input) + } +} + //===----------------------------------------------------------------------===// // Subset parameters //===----------------------------------------------------------------------===// diff --git a/test/AutoDiff/forward_mode_diagnostics.swift b/test/AutoDiff/forward_mode_diagnostics.swift index f249b733ac9c8..62812041fbe1d 100644 --- a/test/AutoDiff/forward_mode_diagnostics.swift +++ b/test/AutoDiff/forward_mode_diagnostics.swift @@ -80,7 +80,8 @@ func one() -> Float { } @differentiable func nonVariedResult(_ x: Float) -> Float { - // expected-warning @+1 2 {{result does not depend on differentiation arguments and will always have a zero derivative; do you want to use 'withoutDerivative(at:)'?}} {{10-10=withoutDerivative(at:}} + // TODO(TF-788): Re-enable non-varied result warning. + // xpected-warning @+1 2 {{result does not depend on differentiation arguments and will always have a zero derivative; do you want to use 'withoutDerivative(at:)'?}} {{10-10=withoutDerivative(at:}} return one() }