Skip to content

Conversation

@dan-zheng
Copy link
Contributor

Initial merge commit contains untouched merge conflicts to facilitate review.


Issues that surfaced in the merge:

  • TF-989: make differentiation transform generate ossa reabstraction thunks.
    This is necessary to re-enable marking the thunks as transparent.
  • TF-990: fix control flow differentiation to work with mergeBasicBlocks
    called during mandatory inlining.

CodaFi and others added 30 commits November 13, 2019 10:06
It's only used from code completion, but it has a dependency on operator lookup so it's gotta stay a member for now
This adjusts the cmake invocation for Foundation to use the export
targets rather than computing the locations by hand.
[OSLogOptimization] Improve the OSLogOptimization pass so that it can fold constant arrays and closure literals
Update rules for the Windows build after CMake 3.15 upgrade.  This simplifies the rules, relies on the export targets and removes the unnecessary variables.
Changed "Numerical computing in Swift Swift is an
expressive..." to "Numerical computing in Swift is an
expressive..."
…teed) transform if all destroys are dead end and we have a local borrow scope.

This is an analogous fix to a previous fix where we were eliminating copies that
were post-dominated by dead ends such that the copy_value did not have any
destroys.

rdar://56807157
EscapeAnalysis: rework graph update and merge algorithms
This property will allow alias analysis to be safely optmistic when
querying the connection graph. A node that is known to have a ref
count is know to keep alive everything it points to. Therefore,
calling a deinitializer on a different reference cannot release the RC
node's contents.
Detect and diagnose a problem when explicitly specified closure
result type doesn't match what is expected by the context:

Example:

```swift
func foo(_: () -> Int) {}

foo { () -> String in "" } // `Int` vs. `String`
```
diagnosing failures in applySolutionFixes, coalesce fixes and
diagnose failures in one method on ConstraintFix.

This eliminates the need for the `DefaultGenericArgument` fix (which
was renamed from `ExplicitlySpecifyGenericArguments`) to have an
array of missing parameters, which was only used when the fixes were
coalesced. Instead, the coalesced arguments are used to create the
`MissingGenericArgumentsFailure` diagnostic directly.
EscapeAnalysis: add a refcount flag to content nodes.
#22235 added an assert that was a little overzealous about
the structure of access control attributes in the AST.  It used to
always expect that we reached diagnoseWitnessFixAccessLevel when there
was an explicit mismatch in user-written access markers.  But declarations
may also have their access levels rewritten and their access attributes
stripped if they are invalid.  If you can get all of these to occur by
the time you diagnose the witness, you see no attribute for a witness
with insufficient ACLs for a requirement and crash.

Relax the assert by using the formal access level and resolve rdar://56818960
build: adjust cmake invocation for export targets
This bit has historically survived typechecking and parsing across source files.  Stick it where we stick the other global state.

This also means we don't have to thread TopLevelContext around anymore when invoking high-level typechecking entrypoints.
It's really not important where it manages to find this unique value, but it's gotta get it from somewhere.  Shameless steal DiscriminatorFinder from DebuggerTransform to scrobble for the next available discriminator value.
…flag in

`TypeVariableOptions` rather than using a separate data structure in the
constraint system.
Explicitly doing a 64bit shift silences the otherwise noisy warning:
```
[1548/1735] Building CXX object
tools\swift\lib\Sema\CMakeFiles\swiftSema.dir\NameBinding.cpp.obj

S:\toolchain\swift\include\swift/AST/IndexSubset.h(96): warning C4334:
'<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit
shift intended?)

S:\toolchain\swift\include\swift/AST/IndexSubset.h(184): warning C4334:
'<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit
shift intended?)
```
[Sanitizers] Add Driver/Frontend option to enable sanitizer instrumentation that supports error recovery
DougGregor and others added 11 commits November 19, 2019 08:46
dlclose is not really well supported with frameworks.
Fix some SourceKit assertion hits
[FrontEnd] Pretty stack trace indicating running user code
build: remove `SWIFT_NEED_EXPLICIT_LIBDISPATCH` (NFC)
Remove test of dynamic replacement and dlclose
…ck-trace

Revert "[FrontEnd] Pretty stack trace indicating running user code"
…erge

Tag build swift-DEVELOPMENT-SNAPSHOT-2019-11-20-a

Committing all conflicts to faciliate reviewing of fixes.
Notes:
- Upstream AutoDiff changes led to code duplication from `master`/`tensorflow`
  branches. I deduped code, moved code to match locations on `master` branch,
  and added `SWIFT_ENABLE_TENSORFLOW` markers in various files, notably
  `include/swift/AST/AutoDiff.h`.
- There are some breakages since `-enable-strip-ownership-after-serialization`
  now defaults to true. Ownership model eliminator no longer runs immediately
  after differentiation. Details below.

Todos:
- TF-989: make differentiation transform generate ossa reabstraction thunks.
  This is necessary to re-enable marking the thunks as transparent.
- TF-990: fix control flow differentiation to work with `mergeBasicBlocks`
  called during mandatory inlining.
@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Nov 23, 2019
`-enable-strip-ownership-after-serialization` is now true by default,
skipping ownership model eliminator after differentiation.

This is problematic for control flow differentiation. `VJPEmitter` creates
trampoline bbs, which create predecessor enum values and immediately branch to
a destination bb. Trampoline bbs share the argument of their destination bb,
which may have `@guaranteed` ownership kind. `@guaranteed` values need a single
lifetime-ending use, e.g. an `end_borrow`.

Previously, `VJPEmitter::trampolinedGuaranteedPhiArguments` tracked all
`@guaranteed` trampoline/destination bb argument pairs and emitted an
`end_borrow` for the trampoline bb argument after the cloned `end_borrow` for
the destination bb argument. However, this does not work when ownership model
eliminator is skipped: mandatory inlining invokes `mergeBasicBlocks` on VJPs,
which causes the `end_borrow` instructions to be redundant.

Now, `VJPEmitter` preemptively calls `mergeBasicBlocks`, which merges trampoline
bbs with their destinations. `@guaranteed` bb arguments are merged and no longer
duplicated, so there is no need to specially emit `end_borrow` for them.

Consequences:
- VJP functions now have simplified CFGs.
- All VJP trampoline bbs are merged with their destinations. Consider rewriting
  the transform to generate the merged destination bbs in the first place.

Resolves TF-990.
…8460)

SR-11336 was recently resolved, making it possible to re-enable the flag.
Re-enable previously disabled SILOptimizer tests.

Resolves TF-799.
@dan-zheng
Copy link
Contributor Author

@swift-ci Please clean test tensorflow

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow linux

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow linux cpu

@dan-zheng dan-zheng requested a review from burmako December 3, 2019 20:24
@dan-zheng
Copy link
Contributor Author

Running extended test suite now - they passed previously before merging tensorflow -> tensorflow-merge, so it's likely they'll continue to pass.

Results expected within two hours.

@dan-zheng
Copy link
Contributor Author

dan-zheng commented Dec 3, 2019

Extended test suite has passed! macOS toolchains continue to build without regressions.
Merging.

@dan-zheng dan-zheng merged commit 49e8835 into tensorflow Dec 3, 2019
@dan-zheng dan-zheng deleted the tensorflow-merge branch December 3, 2019 22:14
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.