Skip to content
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

Jump to bottom

SSACFGBuilder: clean up tryRemoveTrivialPhi #15441

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
+12 −45
Open

SSACFGBuilder: clean up tryRemoveTrivialPhi #15441

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
57 changes: 12 additions & 45 deletions libyul/backends/evm/SSAControlFlowGraphBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,55 +84,36 @@ SSACFG::ValueId SSAControlFlowGraphBuilder::tryRemoveTrivialPhi(SSACFG::ValueId
return _phi; // phi merges at least two distinct values -> not trivial
same = arg;
}

struct Use {
std::reference_wrapper<SSACFG::Operation> operation;
size_t inputIndex = std::numeric_limits<size_t>::max();
};
std::vector<Use> uses;
std::vector<SSACFG::ValueId> phiUses;

if (!same.hasValue())
{
// This will happen for unreachable paths.
// TODO: check how best to deal with this
same = m_graph.unreachableValue();
}

auto phiBlock = phiInfo->block;
m_graph.block(phiBlock).phis.erase(_phi);
m_graph.block(phiInfo->block).phis.erase(_phi);

std::set<SSACFG::ValueId> phiUses;
for (size_t blockIdValue = 0; blockIdValue < m_graph.numBlocks(); ++blockIdValue)
{
auto& block = m_graph.block(SSACFG::BlockId{blockIdValue});
for (auto blockPhi: block.phis)
{
if (blockPhi == _phi)
continue;
auto const* blockPhiInfo = std::get_if<SSACFG::PhiValue>(&m_graph.valueInfo(blockPhi));
yulAssert(blockPhi != _phi, "Phis should be defined in exactly one block, _phi was erased.");
auto* blockPhiInfo = std::get_if<SSACFG::PhiValue>(&m_graph.valueInfo(blockPhi));
yulAssert(blockPhiInfo);
bool usedInPhi = false;
for (auto input: blockPhiInfo->arguments)
{
if (input == _phi)
for (auto& arg: blockPhiInfo->arguments)
if (arg == _phi)
{
arg = same;
usedInPhi = true;
}
}
if (usedInPhi)
phiUses.emplace_back(blockPhi);
phiUses.emplace(blockPhi);
}
for (auto& op: block.operations)
{
// TODO: double check when phiVar occurs in outputs here and what to do about it.
if (op.outputs.size() == 1 && op.outputs.front() == _phi)
continue;
// TODO: check if this always hold - and if so if the assertion is really valuable.
for (auto& output: op.outputs)
yulAssert(output != _phi);
Comment on lines -125 to -130
Copy link
Member Author

Choose a reason for hiding this comment

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

From a conceptual point of view: A phi function value can never be the output of an operation that isn't the phi function itself, as that would violate the SSA form.
From an implementation point of view: Operation outputs are always emplaced as m_graph.newVariable(m_currentBlock) and beyond that not mutated.


for (auto&& [n, input]: op.inputs | ranges::views::enumerate)
if (input == _phi)
uses.emplace_back(Use{std::ref(op), n});
}
std::replace(op.inputs.begin(), op.inputs.end(), _phi, same);
std::visit(util::GenericVisitor{
[_phi, same](SSACFG::BasicBlock::FunctionReturn& _functionReturn) {
std::replace(
Expand All @@ -155,22 +136,8 @@ SSACFG::ValueId SSAControlFlowGraphBuilder::tryRemoveTrivialPhi(SSACFG::ValueId
[](SSACFG::BasicBlock::Terminated&) {}
}, block.exit);
}
for (auto& use: uses)
use.operation.get().inputs.at(use.inputIndex) = same;
for (auto phiUse: phiUses)
{
auto* blockPhiInfo = std::get_if<SSACFG::PhiValue>(&m_graph.valueInfo(phiUse));
yulAssert(blockPhiInfo);
for (auto& arg: blockPhiInfo->arguments)
if (arg == _phi)
arg = same;
}
for (auto& [_, currentVariableDefs]: m_currentDef)
{
for (auto& def: currentVariableDefs)
if (def && *def == _phi)
def = same;
}
std::replace(currentVariableDefs.begin(), currentVariableDefs.end(), _phi, same);

for (auto phiUse: phiUses)
tryRemoveTrivialPhi(phiUse);
Expand Down