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

Remove apply() calls to redundant parsers #3356

Merged
merged 4 commits into from
Jun 25, 2022
Merged

Remove apply() calls to redundant parsers #3356

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7a2fa10
Remove apply() calls to redundant parsers
bleibig May 24, 2022
ec96b7a
Fix cpplint errors
bleibig Jun 3, 2022
bdf5807
Rewrite EliminateSubparserCalls to use MethodInstance
bleibig Jun 9, 2022
13842c2
Run type checking before removing redundant parsers
bleibig Jun 17, 2022
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
Prev Previous commit
Next Next commit
Fix cpplint errors
  • Loading branch information
@bleibig
bleibig committed Jun 16, 2022
commit ec96b7a40ab368155883846c7d72ab72a5bf519a
1 change: 1 addition & 0 deletions frontends/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ set (P4_FRONTEND_HDRS
p4/parserCallGraph.h
p4/parserControlFlow.h
p4/reassociation.h
p4/redundantParsers.h
p4/removeParameters.h
p4/removeReturns.h
p4/reservedWords.h
Expand Down
2 changes: 1 addition & 1 deletion frontends/p4/frontend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P
new RemoveDontcareArgs(&refMap, &typeMap),
new MoveConstructors(&refMap),
new RemoveAllUnusedDeclarations(&refMap),
new ClearTypeMap(&typeMap),
new RemoveRedundantParsers(&typeMap),
new ClearTypeMap(&typeMap),
evaluator,
new Inline(&refMap, &typeMap, evaluator, options.optimizeParserInlining),
new InlineActions(&refMap, &typeMap),
Expand Down
1 change: 1 addition & 0 deletions frontends/p4/redundantParsers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ limitations under the License.
*/

#include "redundantParsers.h"
#include "typeMap.h"

namespace P4 {
bool FindRedundantParsers::preorder(const IR::P4Parser *parser) {
Expand Down
19 changes: 10 additions & 9 deletions frontends/p4/redundantParsers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2013-present Barefoot Networks, Inc.
Copyright 2022-present Barefoot Networks, Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -14,22 +14,23 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

#ifndef _P4_REDUNDANT_PARSERS_H
#define _P4_REDUNDANT_PARSERS_H
#ifndef FRONTENDS_P4_REDUNDANTPARSERS_H_
#define FRONTENDS_P4_REDUNDANTPARSERS_H_

#include "ir/ir.h"
#include "typeMap.h"

namespace P4 {

class TypeMap;

/** Find parsers that have an unconditional "accept" in their start
* state, and put them in redundantParsers.
*/
class FindRedundantParsers : public Inspector {
std::set<cstring> &redundantParsers;
Copy link
Contributor

Choose a reason for hiding this comment

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

why string and not IR::P4Parser?

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 tried using IR::P4Parser *, it didn't quite work because the pointers discovered in RemoveRedundantParsers did not match those found earlier, despite having the same name. Is there supposed to be a single instance of every declaration in the IR?

Copy link
Contributor

Choose a reason for hiding this comment

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

In a transform you should use getOriginal(), there are lots of examples in the code

bool preorder(const IR::P4Parser *parser) override;
public:
FindRedundantParsers(std::set<cstring> &redundantParsers)
explicit FindRedundantParsers(std::set<cstring> &redundantParsers)
: redundantParsers(redundantParsers) { }
};

Expand All @@ -40,16 +41,16 @@ class EliminateSubparserCalls : public Transform {
const std::set<cstring> &redundantParsers;
TypeMap *typeMap;
const IR::Node *postorder(IR::MethodCallStatement *methodCallStmt) override;
public:
public:
EliminateSubparserCalls(const std::set<cstring> &redundantParsers, TypeMap *typeMap)
: redundantParsers(redundantParsers), typeMap(typeMap)
{ }
};

class RemoveRedundantParsers : public PassManager {
std::set<cstring> redundantParsers;
public:
RemoveRedundantParsers(TypeMap *typeMap)
public:
explicit RemoveRedundantParsers(TypeMap *typeMap)
: PassManager {
new FindRedundantParsers(redundantParsers),
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want the 'type' field to be correct then you have to insert a call to typeChecking here. But it's probably simpler in that case to pass the computed typeMap annd use MethodInstance with a typeMap argument.

new EliminateSubparserCalls(redundantParsers, typeMap)
Copy link
Contributor

Choose a reason for hiding this comment

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

The typeMap will always be empty the way you inserted this pass.
Don't you need to invoke the typechecker first?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you do you will need a handle to the referenceMap too.

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 changed it to run before ClearTypeMap. Though from my testing, even before the type map was not empty when this pass was run.

Expand All @@ -60,4 +61,4 @@ class RemoveRedundantParsers : public PassManager {

}

#endif
#endif /* FRONTENDS_P4_REDUNDANTPARSERS_H_ */