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

Migrate some P4Testgen utilities to lib folder. #3630

Merged
merged 4 commits into from
Oct 31, 2022
Merged

Migrate some P4Testgen utilities to lib folder. #3630

merged 4 commits into from
Oct 31, 2022

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Oct 27, 2022

This is in preparation to migrate some files from the P4Tools folder to the compiler library folders.

@fruffy fruffy marked this pull request as ready for review October 27, 2022 21:39
@fruffy
Copy link
Collaborator Author

fruffy commented Oct 27, 2022 •
edited
Loading

Not sure what to do about Extract_Varbits type. It can not be cached or checked since it is part of the Tools IR. This means getConstant either has to moved to the Tools back end or the Extract_Varbits IR type has to be moved to the core IR. Both solutions are not ideal.

@fruffy fruffy force-pushed the fruffy/utils branch 2 times, most recently from dbec75b to fe55008 Compare October 27, 2022 22:36
@fruffy fruffy force-pushed the fruffy/utils branch 4 times, most recently from 33b4b25 to 4bfa685 Compare October 28, 2022 17:49
@fruffy
Patchup.
6ae4255 
cpplint.


Compilation.


Fix typeid.
@fruffy fruffy requested a review from vhavel as a code owner October 28, 2022 20:01
ChrisDodd
ChrisDodd reviewed Oct 30, 2022
View reviewed changes
ir/irutils.h Outdated
//
// Some of these are just thin wrappers around functions in the IR, but it's nice having everything
// in one place.
class IRUtils {
Copy link
Contributor

Choose a reason for hiding this comment

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

since this class has no instances and no non-static members, maybe it should be a namespace?

or maybe even put all these functions in namespace IR directly.

@fruffy fruffy requested a review from mihaibudiu October 31, 2022 13:59
mihaibudiu
mihaibudiu approved these changes Oct 31, 2022
View reviewed changes
backends/p4tools/common/lib/util.cpp

const cstring Utils::Valid = "*valid";

const StateVariable& Utils::getZombieTableVar(const IR::Type* type, const IR::P4Table* table,
Copy link
Contributor

Choose a reason for hiding this comment

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

this part is not really migration

@mihaibudiu mihaibudiu merged commit 48ea120 into main Oct 31, 2022
@mihaibudiu mihaibudiu deleted the fruffy/utils branch October 31, 2022 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrisDodd ChrisDodd ChrisDodd left review comments

@mihaibudiu mihaibudiu mihaibudiu approved these changes

@pkotikal pkotikal Awaiting requested review from pkotikal

@jnfoster jnfoster Awaiting requested review from jnfoster

@vhavel vhavel Awaiting requested review from vhavel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fruffy @ChrisDodd @mihaibudiu