Only emit one copy-to-host or copy-to-accelerator compiler warning per source location, even if it has multiple host uses. #18402
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please test tensorflow |
lattner
reviewed
Jul 31, 2018
| // DenseMap/DenseSet key type instead of SourceRange. That would require that | ||
| // the call-site translate a SILLocation / SourceRange to line and column | ||
| // numbers first. | ||
| template<> struct DenseMapInfo<SourceRange> { |
There was a problem hiding this comment.
This should go in the header that defines SourceRange, and upstreamed :-)
lattner
reviewed
Jul 31, 2018
| // Emit the warning on this value, if that has not been done before. | ||
| if (!copyToAccelWarningLocs.insert(loc.getSourceRange()).second) | ||
| return; | ||
|
|
||
| diagnose(ctx, loc.getSourceLoc(), diagID, description) |
There was a problem hiding this comment.
Do you need to unique based on the entire source range? Would it be fine to just unique based on loc.getSourceLoc() ? That would eliminate the need for template specialization entirely.
There was a problem hiding this comment.
DenseMapInfo is not defined for SourceLoc either, so I might as make SourceRange work as the key type.
Compiler error for using llvm::DenseSet<SourceLoc> copyToHostWarningLocs:
/usr/local/google/home/hongm/ssd_part/git/swift-base/llvm/include/llvm/ADT/DenseMap.h:421:22: error: no member named 'getEmptyKey' in 'llvm::DenseMapInfo<swift::SourceLoc>'
return KeyInfoT::getEmptyKey();
lattner
approved these changes
Jul 31, 2018
rxwei
reviewed
Jul 31, 2018
| // the call-site translate a SILLocation / SourceRange to line and column | ||
| // numbers first. | ||
| template<> struct DenseMapInfo<SourceRange> { | ||
| static SourceRange getEmptyKey() { return SourceRange(); } |
There was a problem hiding this comment.
Consider making the empty key and the tombstone key different?
added 2 commits
July 31, 2018 14:09
…r source location, even if it has multiple host uses.
mhong
pushed a commit
to mhong/swift
that referenced
this pull request
Jul 31, 2018
One use case is using DenseSet to manage a set of source locations, so that we emit at most one compiler diagnostic per location (see swiftlang#18402).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves SR-8412.