Skip to content

Conversation

@computerdisciple
Copy link

GVN for thetas

ThetaGammaInversion,

LastEnumValue // must always be the last enum value, used for iteration
,
Copy link
Owner

Choose a reason for hiding this comment

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

Noise. Please remove

Comment on lines 151 to 152
{ OptimizationId::PartialRedundancyElimination,
OptimizationCommandLineArgument::PartialRedundancyElimination_ },
Copy link
Owner

Choose a reason for hiding this comment

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

Indentation

Comment on lines 425 to 428
return std::make_unique<llvm::DeadNodeElimination>();
case JlmOptCommandLineOptions::OptimizationId::PartialRedundancyElimination:
return std::make_unique<llvm::PartialRedundancyElimination>();
return std::make_shared<llvm::DeadNodeElimination>();
Copy link
Owner

Choose a reason for hiding this comment

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

There are two return statements here. Please fix.

return output_to_gvn_[edge];
}

std::cout << "Logic error: missing input for edge" + ctx_node->DebugString() + std::to_string(ctx_node->GetNodeId());
Copy link
Owner

Choose a reason for hiding this comment

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

Do not just print, but throw exception please.

Comment on lines 66 to 67
jlm/llvm/opt/PartialRedundancyElimination.cpp \
jlm/llvm/opt/gvn.cpp \
Copy link
Owner

Choose a reason for hiding this comment

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

The files are alphabetically ordered.

Comment on lines 221 to 224
std::cout << TR_GRAY << "=================================================" << TR_RESET << std::endl;
std::cout <<TR_CYAN << "Gamma node count:" << stat_gamma_count << TR_RESET << std::endl;
std::cout <<TR_CYAN << "Theta node count:" << stat_theta_count << TR_RESET << std::endl;
std::cout << TR_GRAY << "=================================================" << TR_RESET << std::endl;
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove.


TraverseTopDownRecursively(root, dump_node);
TraverseTopDownRecursively(root, dump_region);
std::cout << TR_PURPLE << "=================================================" << TR_RESET << std::endl;
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove


for (auto kv : thetas_){
auto ic = kv.second.stat_iteration_count;
std::cout << TR_ORANGE << kv.first->DebugString() << kv.first->GetNodeId() << " Iteration count: " << ic << TR_RESET << std::endl;
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove.

for (auto& reg : sn.Subregions())
{
this->TraverseTopDownRecursively(reg, cb);
std::cout << ind() << TR_GRAY << "..........................." << TR_RESET << std::endl;
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove.

Comment on lines 297 to 332
/*
static StateEdgesAt findStateIndices(rvsdg::Node* node)
{
StateEdgesAt edge_indices;
edge_indices.io_state = std::make_pair(0,0);
edge_indices.mem_state = std::make_pair(0,0);
bool found_io_state = false;
bool found_mem_state = false;
for (size_t i = 0 ; i < node->ninputs() ; i++){
if ( rvsdg::is<MemoryStateType>(node->input(i)->Type()) ){
edge_indices.mem_state->first = i;
found_io_state = true;
}
if ( rvsdg::is<IOStateType>(node->input(i)->Type()) ){
edge_indices.io_state->first = i;
found_mem_state = true;
}
}
for (size_t o = 0 ; o < node->noutputs() ; o++){
if ( rvsdg::is<MemoryStateType>(node->output(o)->Type()) ){
edge_indices.mem_state->second = o;
}
if ( rvsdg::is<IOStateType>(node->input(o)->Type()) ){
edge_indices.io_state->second = o;
}
}
if (!found_io_state){edge_indices.io_state = std::nullopt;}
if (!found_mem_state){edge_indices.mem_state = std::nullopt;}
return edge_indices;
}
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Dead code, can be removed, no?

@phate phate requested a review from haved November 6, 2025 04:47
Comment on lines +42 to +48
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Warray-bounds"
#pragma GCC diagnostic ignored "-Wstringop-overflow"
if (operands.size() == 1){
return {operands};
}
#pragma GCC diagnostic pop
Copy link
Owner

Choose a reason for hiding this comment

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

Throw these pragmas out, please

Comment on lines +165 to +171
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Warray-bounds"
#pragma GCC diagnostic ignored "-Wstringop-overflow"
if (operands.size() == 1){
return {operands};
}
#pragma GCC diagnostic pop
Copy link
Owner

Choose a reason for hiding this comment

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

Throw these pragmas out, please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants