Skip to content

Commit 4764120

Browse files
author
Mingsheng Hong
committed
Only emit one copy-to-host or copy-to-accelerator compiler warning per source
location, even if it has multiple host uses.
1 parent 44c18b2 commit 4764120

File tree

4 files changed

+58
-7
lines changed

4 files changed

+58
-7
lines changed

lib/SILOptimizer/Mandatory/TFPartition.cpp

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,31 @@ void BlocksReachingTensorCode::dump() {
602602
// FunctionPartitioner
603603
//===----------------------------------------------------------------------===//
604604

605+
namespace llvm {
606+
template<typename T> struct DenseMapInfo;
607+
608+
// An alternative is to SourceManager::getLineAndColumn() as the
609+
// DenseMap/DenseSet key type instead of SourceRange. That would require that
610+
// the call-site translate a SILLocation / SourceRange to line and column
611+
// numbers first.
612+
template<> struct DenseMapInfo<SourceRange> {
613+
static SourceRange getEmptyKey() { return SourceRange(); }
614+
615+
static SourceRange getTombstoneKey() { return SourceRange(); }
616+
617+
static unsigned getHashValue(const SourceRange &Val) {
618+
return hash_combine(
619+
DenseMapInfo<const void *>::getHashValue(Val.Start.getOpaquePointerValue()),
620+
DenseMapInfo<const void *>::getHashValue(Val.End.getOpaquePointerValue()));
621+
}
622+
623+
static bool isEqual(const SourceRange &LHS,
624+
const SourceRange &RHS) {
625+
return LHS == RHS;
626+
}
627+
};
628+
} // end llvm namespace
629+
605630
namespace {
606631
/// Marking values in the host program need to either be moved, copied, or have
607632
/// their results sent over to the accelerator.
@@ -689,6 +714,11 @@ class TFFunctionPartition {
689714
/// Set of all of the __tf_send calls that silence copy-in warnings.
690715
SmallPtrSet<SILInstruction*, 8> explicitCopyMarkers;
691716

717+
/// Set of source locations where we have issued copy-to-host warnings.
718+
llvm::DenseSet<SourceRange> copyToHostWarningLocs;
719+
/// Set of source locations where we have issued copy-to-accelerator warnings.
720+
llvm::DenseSet<SourceRange> copyToAccelWarningLocs;
721+
692722
struct PartitionedTensorProgram {
693723
// Initialize all members to NULL.
694724
PartitionedTensorProgram() {}
@@ -938,7 +968,10 @@ diagnoseCopyToAccelerator(SILValue value, SILInstruction *user,
938968

939969
auto &ctx = hostFn.getModule().getASTContext();
940970

941-
// Emit the warning.
971+
// Emit the warning on this value, if that has not been done before.
972+
if (!copyToAccelWarningLocs.insert(loc.getSourceRange()).second)
973+
return;
974+
942975
diagnose(ctx, loc.getSourceLoc(), diagID, description)
943976
.highlight(loc.getSourceRange());
944977
auto userLoc = getUserSourceLocation(user);
@@ -973,7 +1006,6 @@ diagnoseCopyToAccelerator(SILValue value, SILInstruction *user,
9731006
/// otherwise emit a warning to tell the programmer that they are doing
9741007
/// something that induces an implicit data transfer into their code.
9751008
void TFFunctionPartition::diagnoseUsesFromHost(SILValue value, SILLocation loc){
976-
bool diagnosed = false;
9771009
for (auto *use : value->getUses()) {
9781010
auto *user = use->getUser();
9791011

@@ -995,8 +1027,8 @@ void TFFunctionPartition::diagnoseUsesFromHost(SILValue value, SILLocation loc){
9951027
if (hostFn.getName() == SWIFT_ENTRY_POINT_FUNCTION)
9961028
continue;
9971029

998-
// Only emit one warning per use.
999-
if (!diagnosed)
1030+
// Only emit one warning per value, even if it has multiple host uses.
1031+
if (copyToHostWarningLocs.insert(loc.getSourceRange()).second)
10001032
diagnoseCopyToHost(value, user, loc);
10011033
}
10021034
}

test/TensorFlow/crashers.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,6 @@ public func testMultiResultOp_send_recv() {
244244
// Accelerator -> Host
245245
_hostOp(x)
246246
x += [[2.0]]
247-
// expected-warning @+2{{implicitly copied to the host}}
248247
// expected-warning @+1{{implicitly copied to the host}}
249248
let results = TensorFlow.Raw.softmaxCrossEntropyWithLogits(features: x, labels: x)
250249
// Accelerator -> Host

test/TensorFlow/diagnostics.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,23 @@ public func testDevice() {
4646
_hostOp(extra)
4747
}
4848

49+
// This loop is unrolled, so we have multiple SIL values for `x`, but there
50+
// should be a single copy-to-host compiler warning.
51+
public func SR8412_CopyToHost() {
52+
for _ in 0...10 {
53+
let x = Tensor(1) // expected-warning {{value implicitly copied to the host}}
54+
_hostOp(x)
55+
}
56+
}
57+
58+
@inline(never)
59+
public func hostFunc() -> Tensor<Float> {
60+
return Tensor<Float>(1.0)
61+
}
62+
63+
public func SR8412_CopyToAccel(a: Int32) {
64+
let x = Tensor<Float>(1.0)
65+
for _ in 0...10 {
66+
let _ = hostFunc() + x // expected-warning {{value implicitly copied to the accelerator}}
67+
}
68+
}

test/TensorFlow/integration.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,8 +458,8 @@ public struct NonInlineMethodExample {
458458
var b = Tensor<Float>(2.0)
459459

460460
@inline(never)
461-
public mutating func mutatingMethod() { // expected-warning 2 {{value implicitly copied}}
462-
a += b // expected-note 2 {{value used here}}
461+
public mutating func mutatingMethod() { // expected-warning {{value implicitly copied}}
462+
a += b // expected-note {{value used here}}
463463
b += a
464464
}
465465
}

0 commit comments

Comments
 (0)