Skip to content
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] Skip implicit GenericTypeParamDecl AST verification. #32343

Conversation

dan-zheng
Copy link
Contributor

@dan-zheng dan-zheng commented Jun 12, 2020

The differentiation transform creates implicit auxiliary struct and enum
declarations, cloning implicit GenericTypeParamDecls from a generic
signature into a flat GenericParamList.

These GenericTypeParamDecls may fail depth verification based on
DC->getGenericContextDepth() because the auxiliary struct and enum
declarations are always flat.

Skipping verification for implicit GenericTypeParamDecls created by the
differentiation transform is the easiest way to fix errors. No other
code paths create implicit GenericTypeParamDecls.

An alternative is to make the differentiation transform properly generated
nested struct and enum declarations. This should fix verification but
bloats the number of generated declarations.


Locally confirmed to fix tensorflow/swift-apis compilation with assertions.
Relevant Azure CI failure:

GenericTypeParamDecl has incorrect depth
Stack dump:
0.	Program arguments: /Users/runner/runners/2.169.1/work/1/toolchain-darwin-x64/Library/Developer/Toolchains/unknown-Asserts-development.xctoolchain/usr/bin/swiftc -frontend -merge-modules -emit-module Sources/x10/CMakeFiles/x10_tensor.dir/swift_bindings/apis/CrossReplicaSum.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/swift_bindings/apis/DeviceScope.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/swift_bindings/apis/RawOpsManual.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/swift_bindings/Device.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/swift_bindings/XLAScalarType.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/swift_bindings/XLATensor.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Bindings/EagerExecution.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Bindings/RawOpsGenerated.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Bindings/RawOpsAugmented.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Bindings/RawOpsDispatching.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Bindings/TFTensorOperation.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/BroadcastingPullback.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/CopyableToDevice.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/DifferentialOperators.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/PythonConversion.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/ShapedArray.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/Execution.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/TensorShape.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/Threading.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/Utilities.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/Tensor.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/TensorHandle.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/DataTypes.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/MixedPrecision.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/Runtime.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/StringTensor.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/TensorGroup.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/ArrayOps.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/LazyTensorContext.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/LazyTensorOperation.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/LazyTensorShapeInference.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/LazyTensorTFFunctionBuilder.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/LazyTensorTrace.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/LazyTensorTraceCache.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Core/Serialization.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layers/Convolutional.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layers/Core.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layers/Dense.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layers/Dropout.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layers/Embedding.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layers/Initialization.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layers/Normalization.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layers/Pooling.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layers/Recurrent.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layers/Sequential.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layers/Upsampling.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Operators/Basic.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Operators/Comparison.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Operators/Dataset.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Operators/Image.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Operators/LinearAlgebra.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Operators/Math.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Operators/NN.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Optimizers/MomentumBased.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Optimizers/Optimizer.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Optimizers/SGD.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/BackwardsCompatibility.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Context.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Exports.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Freezable.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Initializers.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Layer.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/Loss.swift.swiftmodule Sources/x10/CMakeFiles/x10_tensor.dir/__/TensorFlow/StdlibExtensions.swift.swiftmodule -parse-as-library -sil-merge-partial-modules -disable-diagnostic-passes -disable-sil-perf-optzns -target x86_64-apple-macosx10.13 -enable-objc-interop -sdk /Applications/Xcode_11.3.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk -I /Users/runner/runners/2.169.1/work/1/s/tensorflow-swift-apis/Sources/x10 -I /Users/runner/runners/2.169.1/work/1/s/tensorflow-swift-apis/Sources/CTensorFlow -I swift -I /Users/runner/runners/2.169.1/work/1/s/tensorflow-swift-apis/Sources/CX10 -I /Users/runner/runners/2.169.1/work/1/tensorflow/tensorflow-darwin-x64/Library/tensorflow-2.2.0-rc1/usr/include -module-link-name x10_tensor -resource-dir /Users/runner/runners/2.169.1/work/1/b/swift-stdlib/lib/swift -O -D USING_X10_BACKEND -D x10_tensor_EXPORTS -Xllvm -sil-inline-generics -Xllvm -sil-partial-specialization -target-sdk-version 10.15 -emit-module-doc-path swift/x10_tensor.swiftdoc -emit-module-source-info-path swift/x10_tensor.swiftsourceinfo -module-name x10_tensor -o swift/x10_tensor.swiftmodule 
1.	compnerd.org Swift version 5.3-dev (LLVM 7a5fc24b4c, Swift 09c12c9a39)
2.	While verifying GenericTypeParamDecl 'τ_1_0' (in module 'x10_tensor')

Todos:

  • Find a minimal REQUIRES: asserts reproducer and add a unit test.
  • Upstream to master, requesting review from code owners. GenericTypeParamDecl AST verification was recently touched.

The differentiation transform creates implicit auxiliary struct and enum
declarations, cloning implicit `GenericTypeParamDecl`s from a generic
signature into a flat `GenericParamList`.

These `GenericTypeParamDecl`s may fail depth verification based on
`DC->getGenericContextDepth()` because the auxiliary struct and enum
declarations are always flat.

Skipping verification for implicit `GenericTypeParamDecl`s created by the
differentiation transform is the easiest way to fix errors. No other
code paths create implicit `GenericTypeParamDecl`s.

An alternative is to make the differentiation transform properly generated
nested struct and enum declarations. This should fix verification but
bloats the number of generated declarations.
@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Jun 12, 2020
@dan-zheng dan-zheng requested a review from compnerd June 12, 2020 07:06
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Sure, as a work around this seems fine. But we should fix this properly. Let’s get the builds working again though.

@dan-zheng dan-zheng merged commit 292c7c4 into swiftlang:tensorflow Jun 12, 2020
@dan-zheng dan-zheng deleted the autodiff-fix-generic-param-decl-verification branch June 12, 2020 16:27
@astrotuna201
Copy link
Contributor

astrotuna201 commented Jul 30, 2021

Hi, is it possible to merge this into the main branch? Without this patch recent attempt to use _Differentiation from a stock tool chain in a debug build fail, and again trigger "GenericTypeParamDecl has incorrect depth". A minimal reproducer is here: https://github.com/astrotuna201/ASTVerifierReproducer.git

rxwei pushed a commit that referenced this pull request Aug 17, 2021
…cl AST verification failure. (#38745)

Provides reproducer lit test case for AutoDiff implicit auxiliary struct and enum
declarations that clone implicit GenericTypeParamDecls from a generic
signature into a flat GenericParamList, and lead to a compiler assert about GenericTypeParamDecl depth during merge module operation.

Related to #32343.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tensorflow This is for "tensorflow" branch PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants