diff --git a/include/swift/SILOptimizer/Analysis/EscapeAnalysis.h b/include/swift/SILOptimizer/Analysis/EscapeAnalysis.h index 832e40214e64b..c1d405c4d7f42 100644 --- a/include/swift/SILOptimizer/Analysis/EscapeAnalysis.h +++ b/include/swift/SILOptimizer/Analysis/EscapeAnalysis.h @@ -660,7 +660,7 @@ class EscapeAnalysis : public BottomUpIPAnalysis { : F(F), EA(EA), isSummaryGraph(isSummaryGraph) {} /// Returns true if the connection graph is empty. - bool isEmpty() { + bool isEmpty() const { return Values2Nodes.empty() && Nodes.empty() && UsePoints.empty(); } @@ -906,11 +906,10 @@ class EscapeAnalysis : public BottomUpIPAnalysis { /// Dump the connection graph to a DOT file for remote debugging. void dumpCG() const; - /// Checks if the graph is OK. - void verify(bool allowMerge = false) const; + /// Checks if the graph is valid and complete. + void verify() const; - /// Just verifies the graph structure. This function can also be called - /// during the graph is modified, e.g. in mergeAllScheduledNodes(). + /// Just verifies the graph nodes. void verifyStructure(bool allowMerge = false) const; friend struct ::CGForDotView; @@ -1045,8 +1044,9 @@ class EscapeAnalysis : public BottomUpIPAnalysis { /// If \p ai is an optimizable @_semantics("array.uninitialized") call, return /// valid call information. - ArrayUninitCall canOptimizeArrayUninitializedCall(ApplyInst *ai, - ConnectionGraph *conGraph); + ArrayUninitCall + canOptimizeArrayUninitializedCall(ApplyInst *ai, + const ConnectionGraph *conGraph); /// Return true of this tuple_extract is the result of an optimizable /// @_semantics("array.uninitialized") call. @@ -1174,25 +1174,9 @@ class EscapeAnalysis : public BottomUpIPAnalysis { virtual bool needsNotifications() override { return true; } - virtual void verify() const override { -#ifndef NDEBUG - for (auto Iter : Function2Info) { - FunctionInfo *FInfo = Iter.second; - FInfo->Graph.verify(); - FInfo->SummaryGraph.verify(); - } -#endif - } - - virtual void verify(SILFunction *F) const override { -#ifndef NDEBUG - if (FunctionInfo *FInfo = Function2Info.lookup(F)) { - FInfo->Graph.verify(); - FInfo->SummaryGraph.verify(); - } -#endif - } + virtual void verify() const override; + virtual void verify(SILFunction *F) const override; }; } // end namespace swift diff --git a/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp b/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp index 9a7c9ff814832..dbfa9931c9cb1 100644 --- a/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp +++ b/lib/SILOptimizer/Analysis/EscapeAnalysis.cpp @@ -609,7 +609,7 @@ void EscapeAnalysis::ConnectionGraph::mergeAllScheduledNodes() { // To pointsTo-> NodeB (but still has a pointsTo edge to NodeB) while (!ToMerge.empty()) { if (EnableInternalVerify) - verify(/*allowMerge=*/true); + verifyStructure(true /*allowMerge*/); CGNode *From = ToMerge.pop_back_val(); CGNode *To = From->getMergeTarget(); @@ -745,7 +745,6 @@ void EscapeAnalysis::ConnectionGraph::mergeAllScheduledNodes() { From->isMerged = true; if (From->mappedValue) { - // If possible, transfer 'From's mappedValue to 'To' for clarity. Any // values previously mapped to 'From' but not transferred to 'To's // mappedValue must remain mapped to 'From'. Lookups on those values will // find 'To' via the mergeTarget. Dropping a value's mapping is illegal @@ -762,7 +761,7 @@ void EscapeAnalysis::ConnectionGraph::mergeAllScheduledNodes() { From->pointsTo = nullptr; } if (EnableInternalVerify) - verify(/*allowMerge=*/true); + verifyStructure(true /*allowMerge*/); } // As a result of a merge, update the pointsTo field of initialNode and @@ -1574,21 +1573,40 @@ bool CGNode::matchPointToOfDefers(bool allowMerge) const { return true; } -void EscapeAnalysis::ConnectionGraph::verify(bool allowMerge) const { +void EscapeAnalysis::ConnectionGraph::verify() const { #ifndef NDEBUG - verifyStructure(allowMerge); + // Invalidating EscapeAnalysis clears the connection graph. + if (isEmpty()) + return; - // Check graph invariants - for (CGNode *Nd : Nodes) { - // ConnectionGraph invariant #4: For any node N, all paths starting at N - // which consist of only defer-edges and a single trailing points-to edge - // must lead to the same - assert(Nd->matchPointToOfDefers(allowMerge)); - if (Nd->mappedValue && !(allowMerge && Nd->isMerged)) { - assert(Nd == Values2Nodes.lookup(Nd->mappedValue)); - assert(EA->isPointer(Nd->mappedValue)); - // Nodes must always be mapped from the pointer root value. - assert(Nd->mappedValue == EA->getPointerRoot(Nd->mappedValue)); + verifyStructure(); + + // Verify that all pointer nodes are still mapped, otherwise the process of + // merging nodes may have lost information. + for (SILBasicBlock &BB : *F) { + for (auto &I : BB) { + if (isNonWritableMemoryAddress(&I)) + continue; + + if (auto ai = dyn_cast(&I)) { + if (EA->canOptimizeArrayUninitializedCall(ai, this).isValid()) + continue; + } + for (auto result : I.getResults()) { + if (EA->getPointerBase(result)) + continue; + + if (!EA->isPointer(result)) + continue; + + if (!Values2Nodes.lookup(result)) { + llvm::dbgs() << "No CG mapping for "; + result->dumpInContext(); + llvm::dbgs() << " in:\n"; + F->dump(); + llvm_unreachable("Missing escape connection graph mapping"); + } + } } } #endif @@ -1597,6 +1615,7 @@ void EscapeAnalysis::ConnectionGraph::verify(bool allowMerge) const { void EscapeAnalysis::ConnectionGraph::verifyStructure(bool allowMerge) const { #ifndef NDEBUG for (CGNode *Nd : Nodes) { + // Verify the graph structure... if (Nd->isMerged) { assert(Nd->mergeTo); assert(!Nd->pointsTo); @@ -1625,6 +1644,19 @@ void EscapeAnalysis::ConnectionGraph::verifyStructure(bool allowMerge) const { } if (Nd->isInterior()) assert(Nd->pointsTo && "Interior content node requires a pointsTo node"); + + // ConnectionGraph invariant #4: For any node N, all paths starting at N + // which consist of only defer-edges and a single trailing points-to edge + // must lead to the same + assert(Nd->matchPointToOfDefers(allowMerge)); + + // Verify the node to value mapping... + if (Nd->mappedValue && !(allowMerge && Nd->isMerged)) { + assert(Nd == Values2Nodes.lookup(Nd->mappedValue)); + assert(EA->isPointer(Nd->mappedValue)); + // Nodes must always be mapped from the pointer root value. + assert(Nd->mappedValue == EA->getPointerRoot(Nd->mappedValue)); + } } #endif } @@ -1780,8 +1812,8 @@ bool EscapeAnalysis::buildConnectionGraphForDestructor( } EscapeAnalysis::ArrayUninitCall -EscapeAnalysis::canOptimizeArrayUninitializedCall(ApplyInst *ai, - ConnectionGraph *conGraph) { +EscapeAnalysis::canOptimizeArrayUninitializedCall( + ApplyInst *ai, const ConnectionGraph *conGraph) { ArrayUninitCall call; // This must be an exact match so we don't accidentally optimize // "array.uninitialized_intrinsic". @@ -2020,12 +2052,9 @@ void EscapeAnalysis::analyzeInstruction(SILInstruction *I, ConGraph->getNode(cast(I)); return; -#define UNCHECKED_REF_STORAGE(Name, ...) \ - case SILInstructionKind::StrongCopy##Name##ValueInst: #define ALWAYS_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE(Name, ...) \ case SILInstructionKind::Name##RetainInst: \ - case SILInstructionKind::StrongRetain##Name##Inst: \ - case SILInstructionKind::StrongCopy##Name##ValueInst: + case SILInstructionKind::StrongRetain##Name##Inst: #include "swift/AST/ReferenceStorage.def" case SILInstructionKind::DeallocStackInst: case SILInstructionKind::StrongRetainInst: @@ -2043,8 +2072,11 @@ void EscapeAnalysis::analyzeInstruction(SILInstruction *I, case SILInstructionKind::SetDeallocatingInst: case SILInstructionKind::FixLifetimeInst: case SILInstructionKind::ClassifyBridgeObjectInst: - case SILInstructionKind::ValueToBridgeObjectInst: - // These instructions don't have any effect on escaping. + // Early bailout: These instructions never produce a pointer value and + // have no escaping effect on their operands. + assert(!llvm::any_of(I->getResults(), [this](SILValue result) { + return isPointer(result); + })); return; #define ALWAYS_OR_SOMETIMES_LOADABLE_CHECKED_REF_STORAGE(Name, ...) \ @@ -2425,7 +2457,7 @@ void EscapeAnalysis::recompute(FunctionInfo *Initial) { if (BottomUpOrder.wasRecomputedWithCurrentUpdateID(FInfo)) { FInfo->Graph.computeUsePoints(); FInfo->Graph.verify(); - FInfo->SummaryGraph.verify(); + FInfo->SummaryGraph.verifyStructure(); } } } @@ -2766,6 +2798,25 @@ void EscapeAnalysis::handleDeleteNotification(SILNode *node) { } } +void EscapeAnalysis::verify() const { +#ifndef NDEBUG + for (auto Iter : Function2Info) { + FunctionInfo *FInfo = Iter.second; + FInfo->Graph.verify(); + FInfo->SummaryGraph.verifyStructure(); + } +#endif +} + +void EscapeAnalysis::verify(SILFunction *F) const { +#ifndef NDEBUG + if (FunctionInfo *FInfo = Function2Info.lookup(F)) { + FInfo->Graph.verify(); + FInfo->SummaryGraph.verifyStructure(); + } +#endif +} + SILAnalysis *swift::createEscapeAnalysis(SILModule *M) { return new EscapeAnalysis(M); } diff --git a/test/SILOptimizer/escape_analysis.sil b/test/SILOptimizer/escape_analysis.sil index 46af879e675bd..eddfac11b7f09 100644 --- a/test/SILOptimizer/escape_analysis.sil +++ b/test/SILOptimizer/escape_analysis.sil @@ -1844,3 +1844,47 @@ bb3(%4 : $AnyObject): %5 = tuple () return %5 : $() } + +// Test value_to_bridge_object merged with a local reference. A graph node +// must be created for all values involved, and they must point to an +// escaping content node. +// CHECK-LABEL: CG of testValueToBridgeObject +// CHECK-NEXT: Val [ref] %1 Esc: , Succ: (%5.1) +// CHECK-NEXT: Val [ref] %5 Esc: , Succ: (%5.1) +// CHECK-NEXT: Con [int] %5.1 Esc: G, Succ: (%5.2) +// CHECK-NEXT: Con %5.2 Esc: G, Succ: +// CHECK-NEXT: Val [ref] %7 Esc: , Succ: (%5.1), %1, %5 +// CHECK-NEXT: Ret [ref] return Esc: , Succ: %7 +// CHECK-LABEL: End +sil @testValueToBridgeObject : $@convention(thin) (Builtin.Word) -> C { +bb0(%0 : $Builtin.Word): + %1 = alloc_ref $C + cond_br undef, bb1, bb2 + +bb1: + %derived = ref_to_bridge_object %1 : $C, %0 : $Builtin.Word + br bb3(%derived : $Builtin.BridgeObject) + +bb2: + %5 = value_to_bridge_object %0 : $Builtin.Word + br bb3(%5 : $Builtin.BridgeObject) + +bb3(%7 : $Builtin.BridgeObject): + %result = bridge_object_to_ref %7 : $Builtin.BridgeObject to $C + return %result : $C +} + +// Test strong_copy_unowned_value returned. It must be marked escaping, +// not simply "returned", because it's reference is formed from a +// function argument. +// CHECK-LABEL: CG of testStrongCopyUnowned +// CHECK-NEXT: Val [ref] %1 Esc: , Succ: (%1.1) +// CHECK-NEXT: Con [int] %1.1 Esc: G, Succ: (%1.2) +// CHECK-NEXT: Con %1.2 Esc: G, Succ: +// CHECK-NEXT: Ret [ref] return Esc: , Succ: %1 +// CHECK-LABEL: End +sil [ossa] @testStrongCopyUnowned : $@convention(thin) (@guaranteed @sil_unowned Builtin.NativeObject) -> @owned Builtin.NativeObject { +bb0(%0 : @guaranteed $@sil_unowned Builtin.NativeObject): + %1 = strong_copy_unowned_value %0 : $@sil_unowned Builtin.NativeObject + return %1 : $Builtin.NativeObject +}