Skip to content

Conversation

@shanjiang98
Copy link

Introduces a new WhileStatementElimination pass to the Dead Code Elimination framework. This pass identifies 'while' loops with a constantly false condition and removes them entirely.

Introduces a new WhileStatementElimination pass to the Dead Code Elimination
framework. This pass identifies 'while' loops with a constantly false condition
and removes them entirely.

Also includes minor header reordering and adjusts pointer syntax
for consistency within 'pass.cc'.
@google-cla
Copy link

google-cla bot commented Nov 21, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@shanjiang98 shanjiang98 changed the title feat(DeadCodeElimination - While loop): Add WhileStatementElimination in DCE pass feat(DeadCodeElimination - While loop and Function wrapper): Add WhileStatementElimination and UnusedFunctionElimination in DCE pass Nov 22, 2025
@phisiart phisiart self-requested a review November 22, 2025 23:47
"input.js",
"output.generated.txt",
"//maldoca/js/ir:lit_test_files",
"//third_party/maldoca/js/ir:lit_test_files",
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should drop the third_party part, since that is an internal path.

@@ -0,0 +1,1489 @@
// exec:begin
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a test case that just runs DCE, i.e. the input should be the state after dynamic constant propagation.


while(true){
console.log("Should not be eliminated");
} No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can add an empty line at the end to get rid of this warning.

@@ -0,0 +1,7 @@
while(false){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's format this (maybe you have to do it manually).

namespace maldoca {
void IfStatementElimination(mlir::Operation* root_op) {
root_op->walk([&](JshirIfStatementOp op) {
namespace maldoca
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you try putting a clang-format config in the repo?

I found this: https://github.com/kehanXue/google-style-clang-format

Comment on lines +72 to +90
mlir::Region &test_region = op.getTest();
if (test_region.empty())
{
return;
}

auto expr_region_end_op =
llvm::dyn_cast<JsirExprRegionEndOp>(&test_region.front().back());
if (expr_region_end_op == nullptr)
{
return;
}

auto condition_op =
expr_region_end_op.getOperand().getDefiningOp<JsirBooleanLiteralOp>();
if (condition_op == nullptr)
{
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

GetExprRegionOp<JsirBooleanLiteralOp>(op.getTest())

});
}

struct SymbolInfo
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add comments that at least explain what inner and outer mean.

Comment on lines +135 to +137
if (llvm::isa<JsirExprsRegionEndOp>(op) || llvm::isa<JsirExprRegionEndOp>(op) ) {
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

// TODO: We can remove this special handling once the trivia is fixed.

Comment on lines +158 to +159
LOG(INFO) << "Warning: Symbol has references but no definitions: "
<< symbol.name() << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should remove this log if this is not an error, otherwise it might spam the log.

}
if (info.definitions.size() > 1)
{
LOG(INFO) << "Warning: Multiple definitions for symbol: "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you remind us why this happens?

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