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.

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

Fix: prevent copy elimination for statement which does not support constant values #3512

Merged
merged 5 commits into from
Sep 9, 2022

Conversation

kamleshbhalui
Copy link
Contributor

dpdk does not expect operands of some of the statements as constant this pr prevent copy prop and elimination for those stmts.

@@ -355,22 +355,100 @@ CopyPropagationAndElimination::copyPropAndDeadCodeElim(
} else if (auto jc = stmt->to<IR::DpdkJmpLessStatement>()) {
instr.push_back(new IR::DpdkJmpLessStatement(jc->label,
replaceIfCopy(jc->src1, false), replaceIfCopy(jc->src2)));
} else if (auto jc = stmt->to<IR::DpdkJmpLessOrEqualStatement>()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this implementation now has a lot of redundancy, I suspect you could do much better by using a visitor with a preorder rmethod on the base classes. But this will work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to create a new PR for refactoring.

return false;
}

bool preorder(const IR::DpdkBinaryStatement *b) override {
usesInfo[b->src1->toString()]++;
usesInfo[b->src2->toString()]++;
defInfo[b->dst->toString()]++;
// dst and src1 can not be eliminated, because both are same
// and dpdk does not allow src1 to be constant
dontEliminate[b->dst->toString()] = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

it looks to me like this could be a helper function which takes an expression.
that could be useful particularly if later you decide to change the way you represent the "dontEliminate" information.

@mihaibudiu mihaibudiu merged commit 61edee1 into p4lang:main Sep 9, 2022
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