---

yaml
---
r: 277503
b: refs/heads/tensorflow-merge
c: f4998a4
h: refs/heads/master
i:
  277501: 20948e3
  277499: 926a272
  277495: c3cca24
  277487: 43421a2
  277471: a86c550
  277439: 9d0423a
  277375: f02aaf9
  277247: a5367cb
  276991: b955419
  276479: 34835d6

Author: dan-zheng

[refs]

@@ -1124,7 +1124,7 @@ refs/tags/swift-4.2-DEVELOPMENT-SNAPSHOT-2018-10-29-a: 1b087071edaea398480fb778e
 refs/tags/swift-4.2-DEVELOPMENT-SNAPSHOT-2018-10-30-a: 8bc9e108e1480d9217299984e428c601c7aaac75
 refs/tags/swift-4.2.1-RELEASE: 02a6ca969ea1387475b6caeb69c31186df7d30b6
 refs/tags/swift-DEVELOPMENT-SNAPSHOT-2018-11-01-a: 3b0299288f8287094b9ef587f46df54f42a347af
-refs/heads/tensorflow-merge: b6774826a3dbb3e24fe0cdba2350144a10904bfe
+refs/heads/tensorflow-merge: f4998a40fc05876bdbeb2911273024f36357dfed
 refs/heads/TensorFlowLite: b91446471276e37bbfe64767c875f3c7f7102954
 refs/heads/ad-side-effects: 19e0c0de1f59b0929c381925df2e8c72cdf4a728
 refs/heads/add-test-for-asan-compiler-crash: 3cdeecffb47bf28707b299fa2b5bdf0769a4a826

branches/tensorflow-merge/include/swift/AST/DiagnosticsSIL.def

@@ -375,6 +375,13 @@ NOTE(autodiff_nondifferentiable_argument,none,
 NOTE(autodiff_nondifferentiable_result,none,
      "cannot differentiate through a non-differentiable result; do you want to "
      "add '.withoutDerivative()'?", ())
+NOTE(autodiff_noderivative_stored_property,none,
+     "cannot differentiate through a '@noDerivative' stored property; do you "
+     "want to add '.withoutDerivative()'?", ())
+WARNING(autodiff_nonvaried_result_fixit,none,
+        "result does not depend on differentiation arguments and will always "
+        "have a zero derivative; do you want to add '.withoutDerivative()'?",
+        ())
 NOTE(autodiff_global_let_closure_not_differentiable,none,
      "global constant closure is not differentiable", ())
 NOTE(autodiff_cannot_differentiate_global_var_closures,none,

branches/tensorflow-merge/lib/SILOptimizer/Mandatory/Differentiation.cpp

@@ -1331,21 +1331,28 @@ void DifferentiableActivityInfo::analyze(DominanceInfo *di,
           if (isVaried(cai->getSrc(), i))
             recursivelySetVaried(cai->getDest(), i);
         }
-        // Handle `struct_extract`.
-        else if (auto *sei = dyn_cast<StructExtractInst>(&inst)) {
-          if (isVaried(sei->getOperand(), i)) {
-            // If `@noDerivative` exists on the field while the struct is
-            // `@_fieldwiseDifferentiable`, this field is not in the set of
-            // differentiable variables that we want to track the variedness of.
-            auto hasNoDeriv = sei->getField()->getAttrs()
-                .hasAttribute<NoDerivativeAttr>();
-            auto structIsFieldwiseDiffable = sei->getStructDecl()->getAttrs()
-                .hasAttribute<FieldwiseDifferentiableAttr>();
-            if (!(hasNoDeriv && structIsFieldwiseDiffable))
-              for (auto result : inst.getResults())
-                setVaried(result, i);
-          }
-        }
+
+// Handle `struct_extract` and `struct_element_addr` instructions.
+// - If the field is marked `@noDerivative` and belongs to a
+//   `@_fieldwiseDifferentiable` struct, do not set the result as varied because
+//   it is not in the set of differentiable variables.
+// - Otherwise, propagate variedness from operand to result as usual.
+#define PROPAGATE_VARIED_FOR_STRUCT_EXTRACTION(INST) \
+  else if (auto *sei = dyn_cast<INST##Inst>(&inst)) { \
+    if (isVaried(sei->getOperand(), i)) { \
+      auto hasNoDeriv = sei->getField()->getAttrs() \
+          .hasAttribute<NoDerivativeAttr>(); \
+      auto structIsFieldwiseDiffable = sei->getStructDecl()->getAttrs() \
+          .hasAttribute<FieldwiseDifferentiableAttr>(); \
+      if (!(hasNoDeriv && structIsFieldwiseDiffable)) \
+        for (auto result : inst.getResults()) \
+          setVaried(result, i); \
+    } \
+  }
+  PROPAGATE_VARIED_FOR_STRUCT_EXTRACTION(StructExtract)
+  PROPAGATE_VARIED_FOR_STRUCT_EXTRACTION(StructElementAddr)
+#undef VISIT_STRUCT_ELEMENT_INNS
+
         // Handle everything else.
         else {
           for (auto &op : inst.getAllOperands())
@@ -3630,6 +3637,37 @@ class AdjointEmitter final : public SILInstructionVisitor<AdjointEmitter> {
     assert(insertion.second); (void)insertion;
   }
 
+  SILValue getAdjointProjection(SILValue originalProjection) {
+    // Handle `struct_element_addr`.
+    if (auto *seai = dyn_cast<StructElementAddrInst>(originalProjection)) {
+      auto adjBase = getAdjointBuffer(seai->getOperand());
+      auto *cotangentVectorDecl =
+          adjBase.getType().getStructOrBoundGenericStruct();
+      auto cotanFieldLookup =
+          cotangentVectorDecl->lookupDirect(seai->getField()->getName());
+      assert(cotanFieldLookup.size() == 1);
+      auto *cotanField = cast<VarDecl>(cotanFieldLookup.front());
+      return builder.createStructElementAddr(
+         seai->getLoc(), adjBase.getValue(), cotanField);
+    }
+    // Handle `tuple_element_addr`.
+    if (auto *teai = dyn_cast<TupleElementAddrInst>(originalProjection)) {
+      auto adjBase = getAdjointBuffer(teai->getOperand());
+      return builder.createTupleElementAddr(
+          teai->getLoc(), adjBase.getValue(), teai->getFieldNo());
+    }
+    // Handle `begin_access`.
+    if (auto *bai = dyn_cast<BeginAccessInst>(originalProjection)) {
+      auto adjBase = getAdjointBuffer(bai->getOperand());
+      if (errorOccurred)
+        return (bufferMap[originalProjection] = ValueWithCleanup());
+      return builder.createBeginAccess(
+          bai->getLoc(), adjBase, bai->getAccessKind(), bai->getEnforcement(),
+          /*noNestedConflict*/ false, /*fromBuiltin*/ false);
+    }
+    return SILValue();
+  }
+
   ValueWithCleanup &getAdjointBuffer(SILValue originalBuffer) {
     assert(originalBuffer->getType().isAddress());
     assert(originalBuffer->getFunction() == &getOriginal());
@@ -3638,59 +3676,24 @@ class AdjointEmitter final : public SILInstructionVisitor<AdjointEmitter> {
     if (!insertion.second) // not inserted
       return insertion.first->getSecond();
 
-    // Diagnose non-differentiable buffers.
-    if (!originalBuffer->getType().isDifferentiable(getModule())) {
-      getContext().emitNondifferentiabilityError(
-          originalBuffer, getDifferentiationTask());
-      errorOccurred = true;
-      return (bufferMap[originalBuffer] = ValueWithCleanup());
+    // Diagnose `struct_element_addr` instructions to `@noDerivative` fields.
+    if (auto *seai = dyn_cast<StructElementAddrInst>(originalBuffer)) {
+      if (seai->getField()->getAttrs().hasAttribute<NoDerivativeAttr>()) {
+        getContext().emitNondifferentiabilityError(
+            originalBuffer, getDifferentiationTask(),
+            diag::autodiff_noderivative_stored_property);
+        errorOccurred = true;
+        return (bufferMap[originalBuffer] = ValueWithCleanup());
+      }
     }
 
-    // Check whether the original buffer is an address-to-address projection.
-    // If so, recurse until the buffer is such a projection but its operand is
-    // not. Then, get the adjoint buffer of the operand and return a
-    // corresponding projection into it.
-    if (Projection::isAddressProjection(originalBuffer) &&
-        !Projection::isObjectToAddressProjection(originalBuffer)) {
-      // Get operand of the projection (i.e. the base memory).
-      auto *inst = cast<SingleValueInstruction>(originalBuffer);
-      Projection proj(inst);
-      auto loc = inst->getLoc();
-      auto base = inst->getOperand(0);
-      // Get the corresponding projection into the adjoint buffer.
-      SILValue adjProj;
-      auto adjBase = getAdjointBuffer(base);
-      if (proj.getKind() == ProjectionKind::Struct) {
-        auto *origField = proj.getVarDecl(base->getType());
-        auto *cotangentVectorDecl =
-            adjBase.getType().getStructOrBoundGenericStruct();
-        auto cotanFieldLookup =
-            cotangentVectorDecl->lookupDirect(origField->getName());
-        assert(cotanFieldLookup.size() == 1);
-        auto *cotanField = cast<VarDecl>(cotanFieldLookup.front());
-        adjProj = builder.createStructElementAddr(loc, adjBase.getValue(),
-                                                  cotanField);
-      } else {
-        adjProj = proj.createAddressProjection(builder, loc, adjBase.getValue())
-                      .get();
-      }
+    // If the original buffer is a projection, return a corresponding projection
+    // into the adjoint buffer.
+    if (auto adjProj = getAdjointProjection(originalBuffer)) {
       ValueWithCleanup projWithCleanup(
-          adjProj, makeCleanupFromChildren({adjBase.getCleanup()}));
+          adjProj, makeCleanup(adjProj, /*cleanup*/ nullptr));
       return (bufferMap[originalBuffer] = projWithCleanup);
     }
-    // If the original buffer is a `begin_access` instruction, get the adjoint
-    // buffer of its operand and return a corresponding `begin_access` into it.
-    if (auto *bai = dyn_cast<BeginAccessInst>(originalBuffer)) {
-      auto adjBase = getAdjointBuffer(bai->getOperand());
-      if (errorOccurred)
-        return (bufferMap[originalBuffer] = ValueWithCleanup());
-      auto *adjAccess = builder.createBeginAccess(
-          bai->getLoc(), adjBase, bai->getAccessKind(), bai->getEnforcement(),
-          /*noNestedConflict*/ false, /*fromBuiltin*/ false);
-      ValueWithCleanup accessWithCleanup(
-          adjAccess, makeCleanupFromChildren({adjBase.getCleanup()}));
-      return (bufferMap[originalBuffer] = accessWithCleanup);
-    }
 
     // Set insertion point for local allocation builder: before the last local
     // allocation, or at the start of the adjoint entry BB if no local
@@ -3803,6 +3806,17 @@ class AdjointEmitter final : public SILInstructionVisitor<AdjointEmitter> {
     SmallVector<SILValue, 8> origFormalResults;
     collectAllFormalResultsInTypeOrder(original, origFormalResults);
     auto origResult = origFormalResults[task->getIndices().source];
+    // Emit warning if original result is not varied, because it will always have
+    // a zero derivative.
+    if (!activityInfo.isVaried(origResult, task->getIndices().source)) {
+      // Emit fixit if original result has a valid source location.
+      auto sourceLoc = origResult.getLoc().getSourceLoc();
+      if (sourceLoc.isValid()) {
+        getContext()
+            .diagnose(sourceLoc, diag::autodiff_nonvaried_result_fixit)
+            .fixItInsertAfter(sourceLoc, ".withoutDerivative()");
+      }
+    }
 
     builder.setInsertionPoint(adjointEntry);
     if (seed->getType().isAddress()) {
@@ -4229,6 +4243,9 @@ class AdjointEmitter final : public SILInstructionVisitor<AdjointEmitter> {
   }
 
   void visitStructExtractInst(StructExtractInst *sei) {
+    assert(!sei->getField()->getAttrs().hasAttribute<NoDerivativeAttr>() &&
+           "`struct_extract` with `@noDerivative` field should not be "
+           "differentiated; activity analysis should not marked as varied");
     auto loc = sei->getLoc();
     auto &differentiationStrategies =
         getDifferentiationTask()->getStructExtractDifferentiationStrategies();
@@ -4562,6 +4579,17 @@ class AdjointEmitter final : public SILInstructionVisitor<AdjointEmitter> {
                      ValueWithCleanup(adjAccess, makeCleanupFromChildren({})));
   }
 
+#define PROPAGATE_BUFFER_CLEANUP(INST) \
+  void visit##INST##Inst(INST##Inst *inst) { \
+    auto &adjBase = getAdjointBuffer(inst->getOperand()); \
+    auto &adjProj = getAdjointBuffer(inst); \
+    adjProj.setCleanup(makeCleanupFromChildren( \
+        {adjProj.getCleanup(), adjBase.getCleanup()})); \
+  }
+  PROPAGATE_BUFFER_CLEANUP(StructElementAddr)
+  PROPAGATE_BUFFER_CLEANUP(TupleElementAddr)
+#undef PROPAGATE_CLEANUP
+
 #define NOT_DIFFERENTIABLE(INST, DIAG) \
   void visit##INST##Inst(INST##Inst *inst) { \
     getContext().emitNondifferentiabilityError( \
@@ -4587,10 +4615,6 @@ class AdjointEmitter final : public SILInstructionVisitor<AdjointEmitter> {
   NO_ADJOINT(StrongRetainUnowned)
   NO_ADJOINT(DestroyValue)
   NO_ADJOINT(DestroyAddr)
-  // Projection operations have no adjoint visitor.
-  // Corresponding adjoint projections are created in `getAdjointBuffer`.
-  NO_ADJOINT(StructElementAddr)
-  NO_ADJOINT(TupleElementAddr)
 #undef NO_DERIVATIVE
 };
 } // end anonymous namespace

branches/tensorflow-merge/lib/Sema/DerivedConformanceDifferentiable.cpp

@@ -872,7 +872,7 @@ static void checkAndDiagnoseImplicitNoDerivative(TypeChecker &TC,
     if (conformsToDifferentiable && !isConstantProperty)
       continue;
     // Otherwise, add an implicit `@noDerivative` attribute.
-    nominal->getAttrs().add(
+    vd->getAttrs().add(
         new (TC.Context) NoDerivativeAttr(/*Implicit*/ true));
     auto loc =
         vd->getLoc().isValid() ? vd->getLoc() : DC->getAsDecl()->getLoc();

branches/tensorflow-merge/test/AutoDiff/autodiff_diagnostics.swift

@@ -43,6 +43,26 @@ extension S : Differentiable, VectorNumeric {
 // expected-note @+1 {{property is not differentiable}}
 _ = gradient(at: S(p: 0)) { s in 2 * s.p }
 
+struct NoDerivativeProperty : Differentiable {
+  var x: Float
+  @noDerivative var y: Float
+}
+// expected-error @+1 {{function is not differentiable}}
+_ = gradient(at: NoDerivativeProperty(x: 1, y: 1)) { s -> Float in
+  var tmp = s
+  // expected-note @+1 {{cannot differentiate through a '@noDerivative' stored property; do you want to add '.withoutDerivative()'?}}
+  tmp.y = tmp.x
+  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 add '.withoutDerivative()'?}} {{13-13=.withoutDerivative()}}
+  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 add '.withoutDerivative()'?}} {{7-7=.withoutDerivative()}}
+  $0.y
+}
+
 //===----------------------------------------------------------------------===//
 // Function composition
 //===----------------------------------------------------------------------===//
@@ -141,8 +161,6 @@ struct TF_305 : Differentiable {
   @noDerivative let activation: Activation
   @noDerivative let strides: (Int, Int)
 
-  // expected-error @+2 {{function is not differentiable}}
-  // expected-note @+2 {{when differentiating this function definition}}
   @differentiable
   public init(
     filter: Float,
@@ -153,7 +171,7 @@ struct TF_305 : Differentiable {
     self.filter = filter
     self.bias = bias
     self.activation = activation
-    self.strides = strides // expected-note {{expression is not differentiable}}
+    self.strides = strides
   }
 }
 

branches/tensorflow-merge/test/AutoDiff/generics.swift

@@ -49,7 +49,8 @@ struct SupervisedTrainer<Model : Layer> {
   var model: Model
   var lossFunction: @differentiable (Model.Output, Model.Output) -> Float
   func fit(y: Model.Output) {
-     _ = gradient(at: Float(1)) { _ in return lossFunction(y, y) }
+    // expected-warning @+1 {{result does not depend on differentiation arguments and will always have a zero derivative; do you want to add '.withoutDerivative()'?}} {{58-58=.withoutDerivative()}}
+    _ = gradient(at: Float(1)) { _ in return lossFunction(y, y) }
   }
 }
 

branches/tensorflow-merge/test/AutoDiff/simple_math.swift

@@ -183,6 +183,31 @@ SimpleMathTests.test("StructMemberwiseInitializer") {
   expectEqual(2, 𝛁foo)
 }
 
+// Tests TF-319: struct with non-differentiable constant stored property.
+SimpleMathTests.test("StructConstantStoredProperty") {
+  struct TF_319 : Differentiable {
+    var x: Float
+    @noDerivative let constant = Float(2)
+
+    @differentiable
+    init(x: Float) {
+      self.x = x
+    }
+
+    @differentiable(wrt: (self, input))
+    func applied(to input: Float) -> Float {
+      return x * constant * input
+    }
+  }
+  func testStructInit(to input: Float) -> Float {
+    let model = TF_319(x: 10)
+    return model.applied(to: input)
+  }
+  expectEqual(TF_319.CotangentVector(x: 6),
+              gradient(at: TF_319(x: 10), in: { $0.applied(to: 3) }))
+  expectEqual(20, gradient(at: 3, in: testStructInit))
+}
+
 SimpleMathTests.test("StructSideEffects") {
   struct Point : AdditiveArithmetic, Differentiable {
     var x: Float

branches/tensorflow-merge/test/TensorFlowRuntime/model_autodiff_runtime.swift

@@ -60,7 +60,6 @@ public struct Conv2D<Scalar: TensorFlowFloatingPoint>: Layer {
   @noDerivative public let strides: (Int32, Int32)
   @noDerivative public let padding: Padding
 
-  // TODO(TF-309): Add `@differentiable` initializer using assignments, when supported.
   @differentiable
   public init(
     filter: Tensor<Scalar>,
@@ -69,8 +68,11 @@ public struct Conv2D<Scalar: TensorFlowFloatingPoint>: Layer {
     strides: (Int, Int),
     padding: Padding
   ) {
-    self.init(filter: filter, bias: bias, activation: activation,
-              strides: (Int32(strides.0), Int32(strides.1)), padding: padding)
+    self.filter = filter
+    self.bias = bias
+    self.activation = activation
+    self.strides = (Int32(strides.0), Int32(strides.1))
+    self.padding = padding
   }
 
   @differentiable