Partial Exectuer: Track progress per SCC to avoid redundant visits

Using a boolean (madeProgress) to track whether visiting a basic block's
instructions yield new information (new edges, computing a phi value, or
registering values). This boolean is pushed to a stack representing the progress
of the parent's SCCS.

After analyzing the last SCC of a parent, we check if any of its SCCs
made progress. If so, recursion continues until the iteration limit is
hit or the SCCs resolve. Otherwise, when no progress is made across all
SCCS, we stop recursing, mark them as visited, and return to the parent
to continue with its remaining children.

Author: Andres Madrid Ucha

llvm/lib/CheerpWriter/PartialExecuter.cpp

@@ -53,6 +53,7 @@
 #include <map>
 #include <unordered_map>
 #include <vector>
+#include <stack>
 #include "llvm/Cheerp/Utility.h"
 #include "llvm/Cheerp/BuiltinInstructions.h"
 
@@ -301,7 +302,7 @@ class PartialInterpreter : public llvm::Interpreter {
 		}
 		return nullptr;
 	}
-	llvm::BasicBlock* visitBasicBlock(FunctionData& data, llvm::BasicBlock& BB);
+	llvm::BasicBlock* visitBasicBlock(FunctionData& data, llvm::BasicBlock& BB, bool& madeProgress);
 	explicit PartialInterpreter(std::unique_ptr<llvm::Module> M)
 		: llvm::Interpreter(std::move(M), /*preExecute*/false)
 	{
@@ -631,7 +632,7 @@ class PartialInterpreter : public llvm::Interpreter {
 	{
 		return (++functionCounters[F] > 0x1000);
 	}
-	void visitOuter(FunctionData& data, llvm::Instruction& I);
+	void visitOuter(FunctionData& data, llvm::Instruction& I, bool& madeProgress);
 	bool replaceKnownCEs()
 	{
 		if(fullyKnownCEs.empty())
@@ -786,7 +787,7 @@ static void removeEdgeBetweenBlocks(llvm::BasicBlock* from, llvm::BasicBlock* to
 	}
 }
 
-llvm::BasicBlock* PartialInterpreter::visitBasicBlock(FunctionData& data, llvm::BasicBlock& BB)
+llvm::BasicBlock* PartialInterpreter::visitBasicBlock(FunctionData& data, llvm::BasicBlock& BB, bool& madeProgress)
 {
 	ExecutionContext& executionContext = getTopCallFrame();
 	executionContext.CurBB = &BB;
@@ -797,7 +798,7 @@ llvm::BasicBlock* PartialInterpreter::visitBasicBlock(FunctionData& data, llvm::
 		// Note that here we could also execute a Call, and that implies adding a CallFrame
 		// executing there (possibly also in depth)
 		// So getTopCallFrame() has to be called since it will possibly change
-		visitOuter(data, *getTopCallFrame().CurInst++);
+		visitOuter(data, *getTopCallFrame().CurInst++, madeProgress);
 	}
 
 	// Find (if there are enough information) the next BB to be visited
@@ -885,6 +886,7 @@ class FunctionData
 	};
 	llvm::Function& F;
 	std::vector<std::pair<llvm::BasicBlock*, llvm::BasicBlock*>> existingEdges;
+
 	typedef std::vector<Value*> VectorOfArgs;
 
 	llvm::DenseMap<const llvm::BasicBlock*, int> visitCounter;
@@ -990,9 +992,10 @@ class FunctionData
 		assert(currentEE);
 		return *currentEE;
 	}
-	void registerEdge(const llvm::BasicBlock* from, const llvm::BasicBlock* to)
+	bool registerEdge(const llvm::BasicBlock* from, const llvm::BasicBlock* to)
 	{
-		visitedEdges.insert({from, to});
+		auto [it, is_inserted] = visitedEdges.insert({from, to});
+		return is_inserted;
 	}
 	bool hasNoInfo(const VectorOfArgs& arguments) const
 	{
@@ -1178,29 +1181,36 @@ class FunctionData
 	{
 		knownValues[&I].everSkipped = true;
 	}
-	void registerValueForInst(llvm::Instruction& I, const GenericValue& GV, PartialInterpreter::BitMask strongBits)
+	bool registerValueForInst(llvm::Instruction& I, const GenericValue& GV, PartialInterpreter::BitMask strongBits)
 	{
 		// Only support integers for now, to simplify the handling of GVs
 		if(!I.getType()->isIntegerTy())
-			return;
+			return false;
 		// Do not attempt tracking of partially known values
 		if(strongBits != PartialInterpreter::BitMask::ALL)
-			return markInstSkipped(I);
+		{
+			markInstSkipped(I);
+			return false;
+		}
 		auto it = knownValues.find(&I);
 		if(it == knownValues.end())
 		{
 			// Never registered any value before, save the current one
 			knownValues[&I].value = GV;
+			return true;
 		}
 		else if(it->second.everSkipped)
 		{
 			// Nothing we can do anymore
+			return false;
 		}
 		else if(it->second.value.IntVal != GV.IntVal)
 		{
 			// Different value observed, bailout
 			it->second.everSkipped = true;
+			return false;
 		}
+		return false;
 	}
 	void replaceKnownValues() const
 	{
@@ -1363,6 +1373,7 @@ class BasicBlockGroupNode
 	// Implicit tree structure
 	BasicBlockGroupNode* parentNode;
 	std::list<BasicBlockGroupNode> childrenNodes;
+	std::stack<bool> SCCProgress;
 
 	// Other metadata
 	FunctionData& data;
@@ -1439,14 +1450,13 @@ class BasicBlockGroupNode
 	// Do the visit of the BB, with 'from' (possibly nullptr if unknown) as predecessor
 	// Loop backs will be directed to another BBgroup
 	// The visit will return the set of reachable BBs, to be added into visitNext
-	void runVisitBasicBlock(FunctionData& data, llvm::BasicBlock& BB, llvm::SmallVectorImpl<llvm::BasicBlock*>& visitNext)
+	void runVisitBasicBlock(FunctionData& data, llvm::BasicBlock& BB, llvm::SmallVectorImpl<llvm::BasicBlock*>& visitNext, bool& madeProgress)
 	{
 		assert(visitNext.empty());
 
 		PartialInterpreter& interpreter = data.getInterpreter();
 		interpreter.incomingBB = from;
-		BasicBlock* ret = interpreter.visitBasicBlock(data, BB);
-
+		BasicBlock* ret = interpreter.visitBasicBlock(data, BB, madeProgress);
 		if (ret)
 		{
 			visitNext.push_back(ret);
@@ -1506,10 +1516,11 @@ class BasicBlockGroupNode
 				registerEdge(bb, succ);
 		}
 	}
-	void registerEdge(llvm::BasicBlock* from, llvm::BasicBlock* to)
+	bool registerEdge(llvm::BasicBlock* from, llvm::BasicBlock* to)
 	{
-		data.registerEdge(from, to);
+		bool is_inserted = data.registerEdge(from, to);
 		notifySuccessor(from, currIter, to);
+		return is_inserted;
 	}
 	void cleanUp(llvm::BasicBlock* block)
 	{
@@ -1555,12 +1566,40 @@ class BasicBlockGroupNode
 		splitIntoSCCs(childrenNodes, reverseMappingBBToGroup);	//These should be partially ordered with the last one possibly being the replica of the current one
 
 		llvm::SmallVector<llvm::BasicBlock*, 4> visitNext;
+		
+		bool madeProgress = false;
+		runVisitBasicBlock(data, *start, visitNext, madeProgress);
+		for (llvm::BasicBlock* succ : visitNext){
+			if (registerEdge(start, succ))
+				madeProgress = true;
+		}
 
-		// Do the actual visit for start, while populating visitNext
-		runVisitBasicBlock(data, *start, visitNext);
-		for (llvm::BasicBlock* succ : visitNext)
-			registerEdge(start, succ);
-
+		if (parentNode)
+		{
+			// Track progress for this BB into the parent's stack
+			parentNode->SCCProgress.push(madeProgress);
+			if (parentNode->childrenNodes.size() == 1)
+			{
+				// If we are here, it means that we have analyzed all the SCCs of a parent
+				// and we can now check if for the current SCCs, there was any progress made
+				bool foundProgressInStack = false;
+				while (!parentNode->SCCProgress.empty())
+				{
+					if (parentNode->SCCProgress.top())
+						foundProgressInStack = true;
+					parentNode->SCCProgress.pop();
+				}
+				// If we there was progress, we allow recursion to continue on this group of SCCs
+				// until they resolve themselves or they hit the top iteration limit.
+				// Otherwise, when progress has stale, we give up on this group of SCC and allow their parent
+				// to continue with the rest of its children
+				if (!foundProgressInStack)
+				{
+					visitAll();
+					return;
+				}
+			}
+		}
 		// First has been already done
 		childrenNodes.pop_back();
 		while (!childrenNodes.empty())
@@ -1633,7 +1672,7 @@ void FunctionData::actualVisit()
 	groupData.recursiveVisit();
 }
 
-void PartialInterpreter::visitOuter(FunctionData& data, llvm::Instruction& I)
+void PartialInterpreter::visitOuter(FunctionData& data, llvm::Instruction& I, bool& madeProgress)
 {
 	if (PHINode* phi = dyn_cast<PHINode>(&I))
 	{
@@ -1647,6 +1686,7 @@ void PartialInterpreter::visitOuter(FunctionData& data, llvm::Instruction& I)
 			if (isValueComputed(incomingVal))
 			{
 				computedPhisValues.push_back({phi, incomingVal});
+				madeProgress = true;
 				return;
 			}
 		}
@@ -1715,18 +1755,17 @@ void PartialInterpreter::visitOuter(FunctionData& data, llvm::Instruction& I)
 			if (next && data.getVisitCounter(next) < MAX_NUMBER_OF_VISITS_PER_BB)
 			{
 				data.incrementVisitCounter(next);
-
 				// We know where execution should proceed
 				incomingBB = I.getParent();
 				getTopCallFrame().CurBB = next;
 				getTopCallFrame().CurInst = getTopCallFrame().CurBB->begin();
 
 				// Also here we have set the proper state for the execution so we can return
+				madeProgress = true;
 				return;
 			}
 			skip = true;
 		}
-
 		if (addToCounter(I.getFunction()))
 			skip = true;
 
@@ -1736,13 +1775,12 @@ void PartialInterpreter::visitOuter(FunctionData& data, llvm::Instruction& I)
 			//Pop current stack (will happen recursively over the call-stack)
 			popStackFrame();
 			removeFromMaps(getTopCallFrame().Caller);
-
 			return;
 		}
 	}
 	//If we are here it means we have to actually perform the execution via Interpreter
 
-	//Iff it's a call, set up the next stack frame
+	//If it's a call, set up the next stack frame
 	if (CallInst* CI = dyn_cast<CallInst>(&I))
 		forwardArgumentsToNextFrame(*CI);
 
@@ -1752,7 +1790,10 @@ void PartialInterpreter::visitOuter(FunctionData& data, llvm::Instruction& I)
 	BitMask strongBits = computeStronglyKnownBits(I.getOpcode(), I);
 
 	if(isInitialCallFrame())
-		data.registerValueForInst(I, getOperandValue(&I), strongBits);
+	{
+		if (data.registerValueForInst(I, getOperandValue(&I), strongBits))
+			madeProgress = true;
+	}
 
 	if (!isa<CallInst>(I))
 	{