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

[SR-14088] Re-enable SIL verification for differentiable_function and SIL diff witnesses #54770

Open
dan-zheng opened this issue Mar 10, 2020 · 4 comments
Assignees
Labels
AutoDiff compiler The Swift compiler in itself

Comments

@dan-zheng
Copy link
Collaborator

Previous ID SR-14088
Radar rdar://problem/73507243
Original Reporter @dan-zheng
Type Sub-task
Status In Progress
Resolution
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Sub-task
Assignee @rxwei
Priority Medium

md5: 3d2c6f9bc1f6178b50b1c770758eb175

Parent-Task:

  • SR-14087 Make AutoDiff work well with substituted SIL function types

blocks:

  • SR-13021 Flaky AutoDiff tests on Linux: "Bus error (core dumped)"

relates to:

  • SR-14042 Fix differentiable function reabstraction and re-enable tests

Issue Description:

@rxwei
Copy link
Member

rxwei commented Dec 7, 2020

@dan-zheng@marcrasi What remains to be done before they can be re-enabled?

@dan-zheng
Copy link
Collaborator Author

The first step would be to uncomment SILVerifier::checkDifferentiableFunctionInst:

void checkDifferentiableFunctionInst(DifferentiableFunctionInst *dfi) {

The verification condition for expected/actual JVP/VJP operand types can be relaxed to sth like checking for "function type ABI differences".

Re-enabling verification would be great. I think it may expose some latent issues in autodiff tests (like test/AutoDiff/reabstraction.swift) that cause flaky test failures.


Currently, @differentiable function types in SIL are quite difficult to correctly reabstract and transform because expected JVP/VJP types are computed from the original function type. I think your old idea of "creating a new SILDifferentiableFunctionType" where all three types are distinct could still be a good idea. That would also enable optimization/specialization of the original function operand when the JVP/VJP operands cannot be optimized.

@rxwei
Copy link
Member

rxwei commented Dec 8, 2020

Thanks for the context. Disabling verification seemed like an suboptimal solution. There are a lot of weaker things that can be checked here, e.g. the equality of unsubstituted types or at least making sure the indirectness and ownership conventions are the same. Not checking these will lead to memory leaks (which happened to me when I make ownership-related changes) and other errors that are hard to debug.

@rxwei
Copy link
Member

rxwei commented Jan 22, 2021

@swift-ci create

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoDiff compiler The Swift compiler in itself
Projects
None yet
Development

No branches or pull requests

3 participants