Fix optimization record handling #2060
Conversation
|
@swift-ci please test |
rjmansfield
force-pushed
the
fix-opt-record-handling
branch
from
3d9f0e6 to
bb85e95
Compare
rjmansfield
requested review from
DougGregor,
cachemeifyoucan,
compnerd and
owenv
as code owners
|
@swift-ci please test |
|
@swift-ci please test |
rjmansfield
force-pushed
the
fix-opt-record-handling
branch
from
bb85e95 to
248e531
Compare
|
|
||
| // In multi-threaded WMO with -save-optimization-record but no explicit paths or file map entries, | ||
| // pass nil to trigger per-file path generation | ||
| let isMultiThreadedWMOWithAutoGenPaths = !compilerMode.usesPrimaryFileInputs && numThreads > 1 && |
There was a problem hiding this comment.
This scenario adds a decent amount of complexity overall. What do you think of not supporting this flow? That is, rely on the build system to ensure it either specifies a single top-level entry for single-threaded WMO or specifies per-file entries otherwise.
Would there be an existing use case or build system we break if we emit an error on this kind of configuration (single output path flag but per-input opt records)?
There was a problem hiding this comment.
I don't know enough about all the use cases to say how much we'd break if we made this an error. I suspect not a lot, but I was considering cases like: swiftc -wmo -num-threads 2 -save-optimization-record file1.swift file2.swift, or makefiles in embedded projects that use -wmo that might also use -num_threads > 1 with -save-optimization-record. I suspect erroring would mainly break the command line usage, and partial output file map entries, but probably both are edge cases.
The other option would be to add this as a temporary measure with a deprecation warning for multithreaded WMO w/o per file paths, to flush out any misuses? Let me know which you prefer.
There was a problem hiding this comment.
Ah, I see, I hadn't thought through the workflow of just the -save-optimization-record without any explicit path arguments at all.
I agree these are edge cases but it seems more reasonable to keep this around as a shorthand, thanks for explaining.
artemcm
left a comment
artemcm
left a comment
There was a problem hiding this comment.
Just the above comment about lifting up the various checks for single-treaded WMO into a computed property, and otherwise LGTM, thanks.
This infrastructure is needed for both primary file mode and multi-threaded WMO optimization record fixes.
rdar://164884975
Validate that multi-threaded WMO with explicit paths requires one path per source file rdar://164494085
Module-level entries should only apply to single-threaded WMO. Add warning when both explicit -save-optimization-record-path and file map entries exist
rjmansfield
force-pushed
the
fix-opt-record-handling
branch
from
248e531 to
ad91f4a
Compare
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
3 participants
This PR fixes several issues with the optimization record handling and is a continuation of #2054 with multiple commits.
Handles multiple producers error by ensuring each input file gets a unique optimization record output path across all compilation modes.
All the various compilation modes make path selection somewhat complicated so the priority is output file map > explicit flags > derived from input filename
Within output file map entries:
For single-threaded WMO: module-level entry > per-file entry > derived
For multi-threaded WMO: per-file entry > derived (but never module-level)
For non-WMO: per-file entry > derived
If both output file map entries and explicit -save-optimization-record-path flags are specified, the output file map wins and a warning is emitted.
When no output file map entry exists, explicit -save-optimization-record-path flags are used. Otherwise, paths are derived from input filenames.
Several tests were added to cover the various compilation modes and scenarios.
rdar://164884975
rdar://164494085