Skip to content

Commit acc3398

Browse files
committed
Add -allow-critical-edges flag.
Provide a mechanism to gradually migrate unit tests away from allowing critical edges via -allow-critical-edges=false. This will be the default in OSSA very soon, and will hopefully become the default eventually for all SIL stages. Note that not all required optimization pass changes have been committed yet. I have pending changes in: - SimplifyCFG - SILCloner subclasses - EagerSpecializer - ArraySpecialization - LoopUtils - LoopRotate There are multiple reasons we need to disallow critical edges: 1. Most passes do not perform CFG transformations. However, we often need to split critical edges and remember to invalidate all SIL analyses at the end of virtually every pass. This is very innefficient and highly bug prone. 2. Many SIL analysis algorithms needs to reason about CFG edges. Avoiding critical edges leads to far simpler and more efficient designs when edges can be identified by blocks. 3. Handling block arguments on conditional branches create complexity at the lowest level of the SIL interface. This complexity is difficult to abstract over and bleeds until any algorithm that needs to reason about phi operands. It's far easier to work with phis if we can easily recover the phi operand with only a reference to the predecessor block. 4. Attempting to preserve critical edges in high and mid level IR blocks optimizations that otherwise have no business optimizing branches. Branch optimization should always be defered to machine level IR where the most relevant heuristics are employed to remove unconditional branches. If code didn't need to be placed on a critical edges, then a branch optimization can easily remove that code from the critical edge.
1 parent a69502e commit acc3398

File tree

1 file changed

+9
-4
lines changed

1 file changed

+9
-4
lines changed

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ static llvm::cl::opt<bool> VerifyDIHoles(
6565
static llvm::cl::opt<bool> SkipConvertEscapeToNoescapeAttributes(
6666
"verify-skip-convert-escape-to-noescape-attributes", llvm::cl::init(false));
6767

68+
// Allow unit tests to gradually migrate toward -allow-critical-edges=false.
69+
static llvm::cl::opt<bool> AllowCriticalEdges("allow-critical-edges",
70+
llvm::cl::init(true));
71+
6872
// The verifier is basically all assertions, so don't compile it with NDEBUG to
6973
// prevent release builds from triggering spurious unused variable warnings.
7074

@@ -5147,7 +5151,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
51475151
}
51485152

51495153
void verifyBranches(const SILFunction *F) {
5150-
// Verify that there is no non_condbr critical edge.
5154+
// Verify no critical edge.
51515155
auto isCriticalEdgePred = [](const TermInst *T, unsigned EdgeIdx) {
51525156
assert(T->getSuccessors().size() > EdgeIdx && "Not enough successors");
51535157

@@ -5170,10 +5174,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
51705174
const TermInst *TI = BB.getTerminator();
51715175
CurInstruction = TI;
51725176

5173-
// Check for non-cond_br critical edges.
5174-
if (isa<CondBranchInst>(TI)) {
5177+
// FIXME: In OSSA, critical edges will never be allowed.
5178+
// In Lowered SIL, they are allowed on unconditional branches only.
5179+
// if (!(isSILOwnershipEnabled() && F->hasOwnership()))
5180+
if (AllowCriticalEdges && isa<CondBranchInst>(TI))
51755181
continue;
5176-
}
51775182

51785183
for (unsigned Idx = 0, e = BB.getSuccessors().size(); Idx != e; ++Idx) {
51795184
require(!isCriticalEdgePred(TI, Idx),

0 commit comments

Comments
 (0)