Skip to content

[AutoDiff] Add differentiability_witness_function verification. #28505

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 29, 2019
Merged

[AutoDiff] Add differentiability_witness_function verification. #28505

merged 2 commits into from
Nov 29, 2019

Conversation

dan-zheng
Copy link
Contributor

@dan-zheng dan-zheng commented Nov 29, 2019 •
edited
Loading

Add differentiability_witness_function instruction assertions:

  • Must never be constructed with null witness.
  • Can only have explicit type in lowered SIL.

Add differentiability_witness_function instruction verification: check that
type of differentiability_witness_function instruction matches the type of
the witnessed SIL function. This detects issues like #28451 (comment).

@dan-zheng
[AutoDiff] Add differentiability_witness_function verification.
43c91b1 
Assert that `differentiability_witness_function` is never constructed with
null witness.

Verify that type of `differentiability_witness_function` instruction matches
the type of the witnessed SIL function.
@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Nov 29, 2019
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng
Move differentiability_witness_function explicit lowered type test.
da13829 
Move to dedicated lowered type test file:
test/AutoDiff/differentiable_function_inst_lowered.sil.
@dan-zheng
Copy link
Contributor Author

test/AutoDiff/differentiability_witness_function_inst.sil failed because it uses differentiability_witness_function_inst with explicit lowered type (as syntax), but this patch makes explicit types valid only in lowered SIL:

sil-opt: /home/swiftci/jenkins/workspace/swift-PR-TensorFlow-Linux/swift/lib/SIL/SILInstructions.cpp:795: swift::DifferentiabilityWitnessFunctionInst::DifferentiabilityWitnessFunctionInst(swift::SILModule &, swift::SILDebugLocation, swift::DifferentiabilityWitnessFunctionKind, swift::SILDifferentiabilityWitness *, Optional<swift::SILType>): Assertion `module.getStage() == SILStage::Lowered && "Explicit type is valid only in lowered SIL"' failed.
Stack dump:
0.	Program arguments: /home/swiftci/jenkins/workspace/swift-PR-TensorFlow-Linux/Ninja-ReleaseAssert+stdlib-Release/swift-linux-x86_64/bin/sil-opt -target x86_64-unknown-linux-gnu -module-cache-path /home/swiftci/jenkins/workspace/swift-PR-TensorFlow-Linux/Ninja-ReleaseAssert+stdlib-Release/swift-linux-x86_64/swift-test-results/x86_64-unknown-linux-gnu/clang-module-cache /home/swiftci/jenkins/workspace/swift-PR-TensorFlow-Linux/swift/test/AutoDiff/differentiability_witness_function_inst.sil 
1.	With parser at source location: /home/swiftci/jenkins/workspace/swift-PR-TensorFlow-Linux/swift/test/AutoDiff/differentiability_witness_function_inst.sil:64:3
 #0 0x0000000004a1a674 PrintStackTraceSignalHandler(void*) (/home/swiftci/jenkins/workspace/swift-PR-TensorFlow-Linux/Ninja-ReleaseAssert+stdlib-Release/swift-linux-x86_64/bin/sil-opt+0x4a1a674)

Fixed in da13829.

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng dan-zheng requested review from rxwei and marcrasi November 29, 2019 12:12
@dan-zheng dan-zheng mentioned this pull request Nov 29, 2019
Closed
@dan-zheng
Copy link
Contributor Author

Merging, since #28501 mirror was approved.

@dan-zheng dan-zheng merged commit cd4a6ba into swiftlang:tensorflow Nov 29, 2019
@dan-zheng dan-zheng deleted the diff-witness-fn-verification-tf branch November 29, 2019 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rxwei rxwei Awaiting requested review from rxwei

@marcrasi marcrasi Awaiting requested review from marcrasi

Assignees
No one assigned
Labels
tensorflow This is for "tensorflow" branch PRs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@dan-zheng