Skip to content

[AutoDiff] Type-check @differentiable attributes during validation. #27613

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 3 commits into from
Oct 11, 2019
Merged

[AutoDiff] Type-check @differentiable attributes during validation. #27613

merged 3 commits into from
Oct 11, 2019

Conversation

dan-zheng
Copy link
Contributor

Type-check @differentiable attributes during TypeChecker::validateDecl for
all relevant declaration kinds (initializers, subscripts, variables), not just
function declarations.

Resolves TF-888.
TF-789 tracks proper request-based type-checking for @differentiable attribute.

@dan-zheng
[AutoDiff] Type-check @differentiable attributes during validation.
368d8b1 
Type-check `@differentiable` attributes during `TypeChecker::validateDecl` for
all relevant declaration kinds (initializers, subscripts, variables), not just
function declarations.

Resolves TF-888.
TF-789 tracks proper request-based type-checking for `@differentiable` attribute.
@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Oct 10, 2019
@dan-zheng dan-zheng requested review from rxwei and marcrasi October 10, 2019 23:53
@dan-zheng
Fix test.
ac17a00 
The primary file must be named "main" to avoid a linkear error:
```
Undefined symbols for architecture x86_64:
  "_main", referenced from:
     implicit entry/start for main executable
ld: symbol(s) not found for architecture x86_64
```
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng
Copy link
Contributor Author

dan-zheng commented Oct 11, 2019 •
edited
Loading

The newly added test fails on Linux:

Failing Tests (1):
    Swift(linux-x86_64) :: AutoDiff/differentiable_attr_cross_module/main.swift

/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf3cosyS2fFZ: error: undefined reference to 'cosf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf3sinyS2fFZ: error: undefined reference to 'sinf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf3tanyS2fFZ: error: undefined reference to 'tanf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf4acosyS2fFZ: error: undefined reference to 'acosf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf4asinyS2fFZ: error: undefined reference to 'asinf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf4atanyS2fFZ: error: undefined reference to 'atanf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf4coshyS2fFZ: error: undefined reference to 'coshf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf4sinhyS2fFZ: error: undefined reference to 'sinhf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf4tanhyS2fFZ: error: undefined reference to 'tanhf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf5acoshyS2fFZ: error: undefined reference to 'acoshf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf5asinhyS2fFZ: error: undefined reference to 'asinhf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf5atanhyS2fFZ: error: undefined reference to 'atanhf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf3expyS2fFZ: error: undefined reference to 'expf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf4exp2yS2fFZ: error: undefined reference to 'exp2f'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf5exp10yS2fFZ: error: undefined reference to 'powf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf5expm1yS2fFZ: error: undefined reference to 'expm1f'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf3logyS2fFZ: error: undefined reference to 'logf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf4log2yS2fFZ: error: undefined reference to 'log2f'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf5log10yS2fFZ: error: undefined reference to 'log10f'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf5log1pyS2fFZ: error: undefined reference to 'log1pf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf3powyS2f_SftFZ: error: undefined reference to 'powf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf3powyS2f_SitFZ: error: undefined reference to 'powf'
/tmp/lit_tmp_Ch1io5/differentiable_attr_other_module-a40872.o:differentiable_attr_other_module-a40872.o:function $sSf4rootyS2f_SitFZ: error: undefined reference to 'powf'
clang-7: error: linker command failed with exit code 1 (use -v to see invocation)
<unknown>:0: error: link command failed with exit code 1 (use -v to see invocation)

I'm able to reproduce this error on Ubuntu but not on macOS.
Explicitly linking the math library via -lm fixes the issue but that doesn't seem robust.

EDIT: TF-892 tracks this issue. It is related to ElementaryFunctions. I chose to work around it for now.

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng dan-zheng merged commit 68b96a2 into swiftlang:tensorflow Oct 11, 2019
@dan-zheng dan-zheng deleted the TF-888 branch October 11, 2019 06:35
@dan-zheng dan-zheng mentioned this pull request Oct 13, 2019
Merged
dan-zheng added a commit that referenced this pull request Oct 13, 2019
@dan-zheng
[AutoDiff] Add @differentiable attribute SILGen assertion. (#27650)
9c76a29 
Previously, `@differentiable` attribute SILGen lowering was skipped if the
attribute did not have resolved parameter indices. This led to unexpected
and difficult-to-debug bugs like TF-888.

Re-add the assertion for robustness. This is possible after
#27613, which fixed the underlying issue
with parameter indices resolution.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rxwei rxwei rxwei approved these changes

@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.
2 participants
@dan-zheng @rxwei