Skip to content

Conversation

@marcrasi
Copy link

@marcrasi marcrasi commented Dec 7, 2018

Fixes/workarounds for some issues:

  • make vjps noinline and nonspecializable to workaround a problem in deabstraction (more details in comment)
  • remove the PublicNonABI linkage for vjps, because linking fails when noinline vjps are PublicNonABI
  • make getTFCompatibleFuncName able to handle AD__ functions
  • I got a SILVerifier error when running TensorFlow/tensor_autodiff.swift in vjp mode. Adding the default arguments for matmul seems to fix it.

The SILVerifier error was:

SIL verification failed: external declarations of SILFunctions with shared visibility is not allowed: SingleFunction || !hasSharedVisibility(RefF->getLinkage()) || RefF->hasForeignBody()
Verifying instruction:
->   // function_ref default argument 2 of static Raw.matMul<A>(_:_:transposeA:transposeB:)
  %3 = function_ref @$s10TensorFlow3RawO6matMul__10transposeA0F1BAA0A0VyxGAI_AIS2btSjRzAA0aB6ScalarRzlFZfA1_ : $@convention(thin) <τ_0_0 where τ_0_0 : Numeric, τ_0_0 : TensorFlowScalar> () -> Bool // user: %4
     %4 = apply %3<Scalar>() : $@convention(thin) <τ_0_0 where τ_0_0 : Numeric, τ_0_0 : TensorFlowScalar> () -> Bool // user: %8

I believe that my PublicNonABI change will bring back https://bugs.swift.org/browse/SR-8723. I don't have swift set up on a macos right now (and I've only seen this failure happen on macos), so I'll run this PR through macos ci to see what happens. Maybe I can fix this by teaching TBDGen to generate symbols for autodiff associated functions?

@marcrasi marcrasi requested a review from rxwei December 7, 2018 00:05
@marcrasi
Copy link
Author

marcrasi commented Dec 7, 2018

@swift-ci please test tensorflow macos

@rxwei
Copy link
Contributor

rxwei commented Dec 7, 2018

You can also try to remove non-abi linkage from primal/adjoint functions. @bgogul encountered exactly this verifier error yesterday, so this may fix his problem.

@rxwei
Copy link
Contributor

rxwei commented Dec 7, 2018

I believe that my PublicNonABI change will bring back https://bugs.swift.org/browse/SR-8723. I don't have swift set up on a macos right now (and I've only seen this failure happen on macos), so I'll run this PR through macos ci to see what happens. Maybe I can fix this by teaching TBDGen to generate symbols for autodiff associated functions?

Yes. Let's discuss this offline tomorrow.

Copy link
Contributor

Choose a reason for hiding this comment

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

Clean up the empty line?

@marcrasi
Copy link
Author

@swift-ci please test tensorflow

2 similar comments
@marcrasi
Copy link
Author

@swift-ci please test tensorflow

@marcrasi
Copy link
Author

@swift-ci please test tensorflow

@marcrasi
Copy link
Author

@swift-ci please test tensorflow

2 similar comments
@marcrasi
Copy link
Author

@swift-ci please test tensorflow

@marcrasi
Copy link
Author

@swift-ci please test tensorflow

@marcrasi
Copy link
Author

@swift-ci please test tensorflow macos

@marcrasi marcrasi merged commit 9796040 into swiftlang:tensorflow Dec 12, 2018
@marcrasi marcrasi deleted the fix-tensorflow-vjp branch December 12, 2018 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants