Skip to content

Commit f05f992

Browse files
gottesmmgottesmm
committed
[move-only] Combine the address/object checker in the same pass so that we only run cleanups once.
Otherwise, sometimes when the object checker emits a diagnostic and cleans up the IR, some of the cleaned up copies are copies that should have been handled by the address checker. The end result is that the address checker does not emit diagnostics for that IR. I found this problem was exascerbated when writing code for escaping closures. This commit also cleans up the passes in preparation for at a future time moving some of the transformations into the utils folder.
1 parent ecb864c commit f05f992

18 files changed

+883
-455
lines changed

‎include/swift/SILOptimizer/PassManager/Passes.def

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -439,9 +439,11 @@ PASS(BugReducerTester, "bug-reducer-tester",
439439
PASS(AssemblyVisionRemarkGenerator, "assembly-vision-remark-generator",
440440
"Emit assembly vision remarks that provide source level guidance of where runtime calls ended up")
441441
PASS(MoveOnlyObjectChecker, "sil-move-only-object-checker",
442-
"Pass that enforces move only invariants on raw SIL for objects")
442+
"Utility pass that enforces move only invariants on raw SIL for objects for testing purposes")
443443
PASS(MoveOnlyAddressChecker, "sil-move-only-address-checker",
444-
"Pass that enforces move only invariants on raw SIL for addresses")
444+
"Utility pass that enforces move only invariants on raw SIL for addresses for testing purposes")
445+
PASS(MoveOnlyChecker, "sil-move-only-checker",
446+
"Pass that enforces move only invariants on raw SIL for addresses and objects")
445447
PASS(ConsumeOperatorCopyableValuesChecker, "sil-consume-operator-copyable-values-checker",
446448
"Pass that performs checking of the consume operator for copyable values")
447449
PASS(TrivialMoveOnlyTypeEliminator, "sil-trivial-move-only-type-eliminator",

‎lib/SILOptimizer/Mandatory/CMakeLists.txt

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,16 @@ target_sources(swiftSILOptimizer PRIVATE
2424
LowerHopToActor.cpp
2525
MandatoryInlining.cpp
2626
MovedAsyncVarDebugInfoPropagator.cpp
27-
MoveOnlyAddressChecker.cpp
28-
MoveOnlyBorrowToDestructureTransform.cpp
29-
MoveOnlyBorrowToDestructureTransformTester.cpp
27+
MoveOnlyAddressCheckerUtils.cpp
28+
MoveOnlyAddressCheckerTester.cpp
29+
MoveOnlyBorrowToDestructureUtils.cpp
30+
MoveOnlyBorrowToDestructureTester.cpp
3031
MoveOnlyDeinitInsertion.cpp
3132
MoveOnlyDiagnostics.cpp
32-
MoveOnlyObjectChecker.cpp
33+
MoveOnlyObjectCheckerUtils.cpp
34+
MoveOnlyObjectCheckerTester.cpp
35+
MoveOnlyChecker.cpp
36+
MoveOnlyUtils.cpp
3337
NestedSemanticFunctionCheck.cpp
3438
OptimizeHopToExecutor.cpp
3539
PerformanceDiagnostics.cpp

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressChecker.h

Whitespace-only changes.

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerTester.cpp

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
//===--- MoveOnlyAddressCheckerTester.cpp ---------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#define DEBUG_TYPE "sil-move-only-checker"
14+
15+
#include "swift/AST/AccessScope.h"
16+
#include "swift/AST/DiagnosticEngine.h"
17+
#include "swift/AST/DiagnosticsSIL.h"
18+
#include "swift/Basic/Debug.h"
19+
#include "swift/Basic/Defer.h"
20+
#include "swift/Basic/FrozenMultiMap.h"
21+
#include "swift/Basic/SmallBitVector.h"
22+
#include "swift/SIL/ApplySite.h"
23+
#include "swift/SIL/BasicBlockBits.h"
24+
#include "swift/SIL/BasicBlockData.h"
25+
#include "swift/SIL/BasicBlockDatastructures.h"
26+
#include "swift/SIL/BasicBlockUtils.h"
27+
#include "swift/SIL/Consumption.h"
28+
#include "swift/SIL/DebugUtils.h"
29+
#include "swift/SIL/FieldSensitivePrunedLiveness.h"
30+
#include "swift/SIL/InstructionUtils.h"
31+
#include "swift/SIL/MemAccessUtils.h"
32+
#include "swift/SIL/OwnershipUtils.h"
33+
#include "swift/SIL/PrunedLiveness.h"
34+
#include "swift/SIL/SILArgument.h"
35+
#include "swift/SIL/SILArgumentConvention.h"
36+
#include "swift/SIL/SILBasicBlock.h"
37+
#include "swift/SIL/SILBuilder.h"
38+
#include "swift/SIL/SILFunction.h"
39+
#include "swift/SIL/SILInstruction.h"
40+
#include "swift/SIL/SILUndef.h"
41+
#include "swift/SIL/SILValue.h"
42+
#include "swift/SILOptimizer/Analysis/ClosureScope.h"
43+
#include "swift/SILOptimizer/Analysis/DeadEndBlocksAnalysis.h"
44+
#include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
45+
#include "swift/SILOptimizer/Analysis/NonLocalAccessBlockAnalysis.h"
46+
#include "swift/SILOptimizer/PassManager/Transforms.h"
47+
#include "swift/SILOptimizer/Utils/CanonicalizeOSSALifetime.h"
48+
#include "swift/SILOptimizer/Utils/InstructionDeleter.h"
49+
#include "llvm/ADT/DenseMap.h"
50+
#include "llvm/ADT/MapVector.h"
51+
#include "llvm/ADT/PointerIntPair.h"
52+
#include "llvm/ADT/PointerUnion.h"
53+
#include "llvm/ADT/STLExtras.h"
54+
#include "llvm/ADT/SmallBitVector.h"
55+
#include "llvm/ADT/SmallPtrSet.h"
56+
#include "llvm/ADT/SmallVector.h"
57+
#include "llvm/Support/Debug.h"
58+
#include "llvm/Support/ErrorHandling.h"
59+
60+
#include "MoveOnlyAddressCheckerUtils.h"
61+
#include "MoveOnlyBorrowToDestructureUtils.h"
62+
#include "MoveOnlyDiagnostics.h"
63+
#include "MoveOnlyObjectCheckerUtils.h"
64+
#include "MoveOnlyUtils.h"
65+
66+
#include <utility>
67+
68+
using namespace swift;
69+
using namespace swift::siloptimizer;
70+
71+
//===----------------------------------------------------------------------===//
72+
// MARK: Top Level Entrypoint
73+
//===----------------------------------------------------------------------===//
74+
75+
namespace {
76+
77+
class MoveOnlyAddressCheckerTesterPass : public SILFunctionTransform {
78+
void run() override {
79+
auto *fn = getFunction();
80+
81+
// Only run this pass if the move only language feature is enabled.
82+
if (!fn->getASTContext().LangOpts.Features.contains(Feature::MoveOnly))
83+
return;
84+
85+
// Don't rerun diagnostics on deserialized functions.
86+
if (getFunction()->wasDeserializedCanonical())
87+
return;
88+
89+
assert(fn->getModule().getStage() == SILStage::Raw &&
90+
"Should only run on Raw SIL");
91+
LLVM_DEBUG(llvm::dbgs() << "===> MoveOnly Addr Checker. Visiting: "
92+
<< fn->getName() << '\n');
93+
auto *dominanceAnalysis = getAnalysis<DominanceAnalysis>();
94+
DominanceInfo *domTree = dominanceAnalysis->get(fn);
95+
auto *poa = getAnalysis<PostOrderAnalysis>();
96+
97+
DiagnosticEmitter diagnosticEmitter;
98+
SmallSetVector<MarkMustCheckInst *, 32> moveIntroducersToProcess;
99+
searchForCandidateAddressMarkMustChecks(fn, moveIntroducersToProcess,
100+
diagnosticEmitter);
101+
102+
LLVM_DEBUG(llvm::dbgs()
103+
<< "Emitting diagnostic when checking for mark must check inst: "
104+
<< (diagnosticEmitter.getDiagnosticCount() ? "yes" : "no")
105+
<< '\n');
106+
107+
bool madeChange = false;
108+
unsigned diagCount = 0;
109+
if (moveIntroducersToProcess.empty()) {
110+
LLVM_DEBUG(llvm::dbgs() << "No move introducers found?!\n");
111+
} else {
112+
borrowtodestructure::IntervalMapAllocator allocator;
113+
MoveOnlyAddressChecker checker{getFunction(), diagnosticEmitter,
114+
allocator, domTree, poa};
115+
madeChange = checker.check(moveIntroducersToProcess);
116+
diagCount = checker.diagnosticEmitter.getDiagnosticCount();
117+
}
118+
119+
// If we did not emit any diagnostics, emit a diagnostic if we missed any
120+
// copies.
121+
if (!diagCount) {
122+
emitCheckerMissedCopyOfNonCopyableTypeErrors(getFunction(),
123+
diagnosticEmitter);
124+
}
125+
126+
// Then cleanup any copies we left behind for either reason and emit an
127+
// error.
128+
madeChange |=
129+
cleanupNonCopyableCopiesAfterEmittingDiagnostic(getFunction());
130+
131+
if (madeChange) {
132+
invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
133+
}
134+
}
135+
};
136+
137+
} // anonymous namespace
138+
139+
SILTransform *swift::createMoveOnlyAddressChecker() {
140+
return new MoveOnlyAddressCheckerTesterPass();
141+
}

0 commit comments

Comments
 (0)