Skip to content

PartialExecuter: Dominators optimization (draft) #357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
andmadri wants to merge 1 commit into from
Closed

PartialExecuter: Dominators optimization (draft) #357

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 69 additions & 28 deletions llvm/lib/CheerpWriter/PartialExecuter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include "llvm/Cheerp/DeterministicUnorderedSet.h"
#include "llvm/Cheerp/PartialExecuter.h"
#include "llvm/ExecutionEngine/GenericValue.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/Operator.h"
#include "llvm/IR/InstIterator.h"
#include "llvm/IR/ValueMap.h"
Expand Down Expand Up @@ -304,6 +305,7 @@ class PartialInterpreter : public llvm::Interpreter {
}
return nullptr;
}
bool checkDominator(FunctionData& data, llvm::BasicBlock* BB);
llvm::BasicBlock* visitBasicBlock(FunctionData& data, BasicBlockGroupNode& BBGN, llvm::BasicBlock& BB, bool& BBProgress);
explicit PartialInterpreter(std::unique_ptr<llvm::Module> M)
: llvm::Interpreter(std::move(M), /*preExecute*/false)
Expand Down Expand Up @@ -392,7 +394,7 @@ class PartialInterpreter : public llvm::Interpreter {
return stronglyKnownBits.back().count(V);
return false;
}
bool areOperandsComputed(const llvm::Instruction& I)
bool areOperandsComputed(const llvm::Instruction& I, bool hasKnownValues)
{
if (isa<SelectInst>(I))
{
Expand All @@ -413,6 +415,8 @@ class PartialInterpreter : public llvm::Interpreter {
}
for (auto& op : I.operands())
{
if (!hasKnownValues && (!isa<llvm::Constant>(op) && !isa<llvm::Argument>(op) && !isa<BasicBlock>(op)))

@alexp-sssup alexp-sssup Apr 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can check for isa directly here

return false;
if (!isValueComputed(op))
return false;
}
Expand All @@ -436,10 +440,10 @@ class PartialInterpreter : public llvm::Interpreter {
newAlignmentData.clear();
}
//We are going to interpret a CallInst, we need to add a stack frame and forward the known arguments
void forwardArgumentsToNextFrame(CallInst& CI)
void forwardArgumentsToNextFrame(CallInst& CI, bool hasKnownValues)
{
//This logic should work similarly for generic CallBase when those are allowed
assert(!hasToBeSkipped(CI));
assert(!hasToBeSkipped(CI, hasKnownValues));

const Function& calledFunc = *cast<Function>(CI.getCalledFunction());
const uint32_t numFixedParams = calledFunc.getFunctionType()->getNumParams();
Expand All @@ -463,9 +467,9 @@ class PartialInterpreter : public llvm::Interpreter {
assignToMaps(pair.first, pair.second, nullptr);
}

bool hasToBeSkipped(llvm::Instruction& I)
bool hasToBeSkipped(llvm::Instruction& I, bool hasKnownValues)
{
if (!areOperandsComputed(I))
if (!areOperandsComputed(I, hasKnownValues))
return true;

switch (I.getOpcode())
Expand Down Expand Up @@ -641,7 +645,7 @@ class PartialInterpreter : public llvm::Interpreter {
}
return nullptr;
}
void visitOuter(FunctionData& data, BasicBlockGroupNode& BBGN, llvm::Instruction& I, bool& BBProgress);
void visitOuter(FunctionData& data, BasicBlockGroupNode& BBGN, llvm::Instruction& I, bool& BBProgress, bool& hasKnownValues);
bool replaceKnownCEs()
{
if(fullyKnownCEs.empty())
Expand Down Expand Up @@ -796,25 +800,6 @@ static void removeEdgeBetweenBlocks(llvm::BasicBlock* from, llvm::BasicBlock* to
}
}

llvm::BasicBlock* PartialInterpreter::visitBasicBlock(FunctionData& data, BasicBlockGroupNode& BBGN, llvm::BasicBlock& BB, bool& BBProgress)

@alexp-sssup alexp-sssup Apr 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid this confusing code movement

{
ExecutionContext& executionContext = getTopCallFrame();
executionContext.CurBB = &BB;
executionContext.CurInst = executionContext.CurBB->begin();

while (getTopCallFrame().CurInst != BB.end())
{
// 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, BBGN, *getTopCallFrame().CurInst++, BBProgress);
}

// Find (if there are enough information) the next BB to be visited
// nullptr otherwise means failure to do so, and will fall back to visit all successors
return findNextBasicBlock(*BB.getTerminator());
}

class FunctionData;

class ModuleData
Expand Down Expand Up @@ -901,8 +886,11 @@ class FunctionData
llvm::DenseSet<llvm::BasicBlock*> visitedBasicBlocks;
llvm::DenseSet<std::pair<const llvm::BasicBlock*, const llvm::BasicBlock*> > visitedEdges;
llvm::DenseMap<llvm::Instruction*, KnownValue> knownValues;
llvm::DenseSet<llvm::BasicBlock*> blockHasKnownValues;
ModuleData& moduleData;

llvm::DominatorTree DT;

std::vector<VectorOfArgs> callEquivalentQueue;
PartialInterpreter* currentEE;
uint32_t functionInstructionCounter;
Expand Down Expand Up @@ -983,11 +971,21 @@ class FunctionData
explicit FunctionData(llvm::Function& F, ModuleData& moduleData)
: F(F), moduleData(moduleData), currentEE(nullptr), functionInstructionCounter(0)
{
if (!F.isDeclaration())
DT.recalculate(F);

@alexp-sssup alexp-sssup Apr 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there might be a better way by declaring to LLVM that this pass depends on DT and DT will make the analysis available. @yuri91 does this make sense?

}
llvm::Function* getFunction()
{
return &F;
}
llvm::DominatorTree& getDT()
{
return DT;
}
llvm::DenseSet<llvm::BasicBlock*>& getBlockHasKnownValues()
{
return blockHasKnownValues;
}
uint32_t getVisitCounter(const llvm::BasicBlock* BB)
{
return visitCounter[BB];
Expand All @@ -1008,6 +1006,10 @@ class FunctionData
{
visitedBasicBlocks.insert(BB);
}
void markBlockHasKnownValues(llvm::BasicBlock* BB)
{
blockHasKnownValues.insert(BB);
}
PartialInterpreter& getInterpreter()
{
// currentEE will be non-null while visiting a Call Equivalent
Expand Down Expand Up @@ -1142,6 +1144,7 @@ class FunctionData
for (const FunctionData::VectorOfArgs& toBeVisited : callEquivalentQueue)
{
visitCounter.clear();
blockHasKnownValues.clear();
visitCallEquivalent(toBeVisited);
if (visitedBasicBlocks.size() == F.size())
break;
Expand Down Expand Up @@ -1254,6 +1257,39 @@ class FunctionData
}
};

llvm::BasicBlock* PartialInterpreter::visitBasicBlock(FunctionData& data, BasicBlockGroupNode& BBGN, llvm::BasicBlock& BB, bool& BBProgress)
{
ExecutionContext& executionContext = getTopCallFrame();
executionContext.CurBB = &BB;
executionContext.CurInst = executionContext.CurBB->begin();

bool hasKnownValues = checkDominator(data, &BB);

@alexp-sssup alexp-sssup Apr 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that the are some similarities to the BBProgress flag, but it's only superficial I think. Knowing an instruction is progress, but not all types of progress imply knowing an instruction

while (getTopCallFrame().CurInst != BB.end())
{
// 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, BBGN, *getTopCallFrame().CurInst++, BBProgress, hasKnownValues);
}
if (hasKnownValues)
data.markBlockHasKnownValues(&BB);
// Find (if there are enough information) the next BB to be visited
// nullptr otherwise means failure to do so, and will fall back to visit all successors
return findNextBasicBlock(*BB.getTerminator());
}

bool PartialInterpreter::checkDominator(FunctionData& data, llvm::BasicBlock* BB)
{
if (auto* DomNode = data.getDT().getNode(BB)) {
if (auto* IDom = DomNode->getIDom()) {
if (data.getBlockHasKnownValues().count(IDom->getBlock())) {
return true;
}
}
}
return false;
}

void ModuleData::initFunctionData()
{
assert(functionData.empty());
Expand Down Expand Up @@ -1618,6 +1654,7 @@ class BasicBlockGroupNode
parentNode->cleanUp(p.first);
}
}

data.incrementVisitCounter(start);

splitIntoSCCs(childrenNodes, reverseMappingBBToGroup); //These should be partially ordered with the last one possibly being the replica of the current one
Expand Down Expand Up @@ -1719,7 +1756,7 @@ void FunctionData::actualVisit()
groupData.recursiveVisit();
}

void PartialInterpreter::visitOuter(FunctionData& data, BasicBlockGroupNode& BBGN, llvm::Instruction& I, bool& BBProgress)
void PartialInterpreter::visitOuter(FunctionData& data, BasicBlockGroupNode& BBGN, llvm::Instruction& I, bool& BBProgress, bool& hasKnownValues)
{
data.incrementFunctionInstructionCounter();
BBGN.incrementSCCInstructionCounter();
Expand All @@ -1737,6 +1774,7 @@ void PartialInterpreter::visitOuter(FunctionData& data, BasicBlockGroupNode& BBG
{
computedPhisValues.push_back({phi, incomingVal});
BBProgress = true;
hasKnownValues = true;
return;
}
}
Expand All @@ -1761,7 +1799,7 @@ void PartialInterpreter::visitOuter(FunctionData& data, BasicBlockGroupNode& BBG

//Here PHI have been properly processed

bool skip = hasToBeSkipped(I);
bool skip = hasToBeSkipped(I, hasKnownValues);

if (isInitialCallFrame())
{
Expand Down Expand Up @@ -1847,7 +1885,7 @@ void PartialInterpreter::visitOuter(FunctionData& data, BasicBlockGroupNode& BBG
assignToMaps(&I, strongBits, ptrBase);
return;
}
forwardArgumentsToNextFrame(*CI);
forwardArgumentsToNextFrame(*CI, hasKnownValues);
}

//Dispatch to the Interpreter's visitor for the given Instructon
Expand All @@ -1858,7 +1896,10 @@ void PartialInterpreter::visitOuter(FunctionData& data, BasicBlockGroupNode& BBG
if(isInitialCallFrame())
{
if (data.registerValueForInst(I, getOperandValue(&I), strongBits))
{
hasKnownValues = true;
BBProgress = true;
}
}

if (!isa<CallInst>(I))
Expand Down
Loading