Skip to content

Commit b0bda13

Browse files
committed
LoadBorrowInvalidation: fix mysteriously inverted boolean returns.
1 parent a790439 commit b0bda13

File tree

3 files changed

+12
-17
lines changed

3 files changed

+12
-17
lines changed

lib/SIL/Verifier/LoadBorrowInvalidationChecker.cpp

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ bool LoadBorrowNeverInvalidatedAnalysis::
322322
// Treat None as a write.
323323
if (!writes) {
324324
llvm::errs() << "Failed to find cached writes for: " << *address;
325-
return false;
325+
return true;
326326
}
327327

328328
auto lbiProjPath =
@@ -351,13 +351,13 @@ bool LoadBorrowNeverInvalidatedAnalysis::
351351
// our address from lbiProjPath. If not, we have to a
352352
if (!lbiProjPath) {
353353
llvm::errs() << "Couldn't find path root for load_borrow: " << *lbi;
354-
return false;
354+
return true;
355355
}
356356

357357
auto writePath = ProjectionPath::getProjectionPath(address, op->get());
358358
if (!writePath) {
359359
llvm::errs() << "Couldn't find path root for write: " << *op->getUser();
360-
return false;
360+
return true;
361361
}
362362

363363
// The symmetric difference of two projection paths consists of the parts of
@@ -368,11 +368,11 @@ bool LoadBorrowNeverInvalidatedAnalysis::
368368
}
369369

370370
llvm::errs() << "Write: " << *op->getUser();
371-
return false;
371+
return true;
372372
}
373373

374374
// Ok, we are good.
375-
return true;
375+
return false;
376376
}
377377

378378
//===----------------------------------------------------------------------===//
@@ -450,8 +450,7 @@ bool LoadBorrowNeverInvalidatedAnalysis::
450450

451451
return false;
452452
}
453-
454-
bool LoadBorrowNeverInvalidatedAnalysis::isNeverInvalidated(
453+
bool LoadBorrowNeverInvalidatedAnalysis::isInvalidated(
455454
LoadBorrowInst *lbi) {
456455

457456
// FIXME: To be reenabled separately in a follow-on commit.
@@ -470,7 +469,7 @@ bool LoadBorrowNeverInvalidatedAnalysis::isNeverInvalidated(
470469

471470
// If we have a let address, then we are already done.
472471
if (storage.isLetAccess()) {
473-
return true;
472+
return false;
474473
}
475474
// At this point, we know that we /may/ have writes. Now we go through various
476475
// cases to try and exhaustively identify if those writes overlap with our
@@ -484,7 +483,7 @@ bool LoadBorrowNeverInvalidatedAnalysis::isNeverInvalidated(
484483
if (auto *bai = dyn_cast<BeginAccessInst>(address)) {
485484
// We do not have a modify, assume we are correct.
486485
if (bai->getAccessKind() != SILAccessKind::Modify) {
487-
return true;
486+
return false;
488487
}
489488

490489
// Otherwise, validate that any writes to our begin_access is not when the
@@ -533,12 +532,8 @@ bool LoadBorrowNeverInvalidatedAnalysis::isNeverInvalidated(
533532
}
534533
case AccessedStorage::Argument: {
535534
auto *arg = cast<SILFunctionArgument>(storage.getArgument());
536-
// We return false if visit a non-address here. Object args are things like
537-
// pointer_to_address that we handle earlier.
538-
if (!arg->getType().isAddress())
539-
return false;
540535
if (arg->hasConvention(SILArgumentConvention::Indirect_In_Guaranteed))
541-
return true;
536+
return false;
542537
return doesAddressHaveWriteThatInvalidatesLoadBorrow(lbi, endBorrowUses,
543538
arg);
544539
}
@@ -547,7 +542,7 @@ bool LoadBorrowNeverInvalidatedAnalysis::isNeverInvalidated(
547542
//
548543
// FIXME: The yielded address could overlap with another address in this
549544
// function.
550-
return true;
545+
return false;
551546
}
552547
case AccessedStorage::Box: {
553548
return doesAddressHaveWriteThatInvalidatesLoadBorrow(lbi, endBorrowUses,

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1881,7 +1881,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
18811881
requireSameType(LBI->getOperand()->getType().getObjectType(),
18821882
LBI->getType(),
18831883
"Load operand type and result type mismatch");
1884-
require(loadBorrowNeverInvalidatedAnalysis.isNeverInvalidated(LBI),
1884+
require(!loadBorrowNeverInvalidatedAnalysis.isInvalidated(LBI),
18851885
"Found load borrow that is invalidated by a local write?!");
18861886
}
18871887

lib/SIL/Verifier/VerifierPrivate.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class LoadBorrowNeverInvalidatedAnalysis {
3434

3535
/// Returns true if exhaustively lbi is guaranteed to never be invalidated by
3636
/// local writes.
37-
bool isNeverInvalidated(LoadBorrowInst *lbi);
37+
bool isInvalidated(LoadBorrowInst *lbi);
3838

3939
private:
4040
bool doesAddressHaveWriteThatInvalidatesLoadBorrow(

0 commit comments

Comments
 (0)