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

Remove apply() calls to redundant parsers #3356

Merged
merged 4 commits into from
Jun 25, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Rewrite EliminateSubparserCalls to use MethodInstance
Also change the parser tracking to use IR::P4Parser pointers
  • Loading branch information
bleibig committed Jun 16, 2022
commit bdf58074f92a655980e6d1186d453fc4294df392
2 changes: 1 addition & 1 deletion frontends/p4/frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P
new RemoveDontcareArgs(&refMap, &typeMap),
new MoveConstructors(&refMap),
new RemoveAllUnusedDeclarations(&refMap),
new RemoveRedundantParsers(&typeMap),
new RemoveRedundantParsers(&refMap),
new ClearTypeMap(&typeMap),
evaluator,
new Inline(&refMap, &typeMap, evaluator, options.optimizeParserInlining),
Expand Down
49 changes: 28 additions & 21 deletions frontends/p4/redundantParsers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ limitations under the License.
*/

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

#include "frontends/common/resolveReferences/referenceMap.h"
#include "frontends/p4/methodInstance.h"

namespace P4 {
bool FindRedundantParsers::preorder(const IR::P4Parser *parser) {
Expand All @@ -30,30 +32,35 @@ bool FindRedundantParsers::preorder(const IR::P4Parser *parser) {
continue;
}
LOG4("Found redundant parser " << parser->name);
redundantParsers.insert(parser->type->name);
redundantParsers.insert(parser);
}
return false;
}

const IR::Node *EliminateSubparserCalls::postorder(IR::MethodCallStatement *mcs) {
auto method = mcs->methodCall->method;
auto *member = method->to<IR::Member>();
if (!member || member->member != IR::IApply::applyMethodName) {
return mcs;
}
auto type = typeMap->getType(member->expr);
if (!type) {
return mcs;
}
auto parser = type->to<IR::Type_Parser>();
if (!parser) {
return mcs;
}
if (redundantParsers.count(parser->name)) {
LOG4("Removing apply call to redundant parser " << parser->getName()
<< ": " << *mcs);
return nullptr;
}
return mcs;
if (mcs->methodCall->method->type->is<IR::Type_Unknown>()) return mcs;
Copy link
Contributor

Choose a reason for hiding this comment

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

In general the type field of an expression is not to be trusted unless you just set it up yourself (probably by calling typeChecking with a 'true' flag).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to run type checking before the new passes

auto mem = mcs->methodCall->method->to<IR::Member>();
if (!mem) return mcs;

auto pe = mem->expr->to<IR::PathExpression>();
if (!pe || !pe->type->is<IR::IApply>()) return mcs;

auto mi = MethodInstance::resolve(mcs->methodCall, refMap, nullptr, true);
if (!mi->isApply()) return mcs;

auto apply = mi->to<ApplyMethod>()->applyObject;
auto parser = apply->to<IR::Type_Parser>();
if (!parser) return mcs;

auto declInstance = mi->object->to<IR::Declaration_Instance>();
if (!declInstance) return mcs;

auto decl = refMap->getDeclaration(declInstance->type->to<IR::Type_Name>()->path);
auto p4parser = decl->to<IR::P4Parser>();
if (!p4parser || !redundantParsers.count(p4parser)) return mcs;

LOG4("Removing apply call to redundant parser " << parser->getName()
<< ": " << *mcs);
return nullptr;
}
}
21 changes: 11 additions & 10 deletions frontends/p4/redundantParsers.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,39 +21,40 @@ limitations under the License.

namespace P4 {

class TypeMap;
class ReferenceMap;

/** Find parsers that have an unconditional "accept" in their start
* state, and put them in redundantParsers.
*/
class FindRedundantParsers : public Inspector {
std::set<cstring> &redundantParsers;
std::set<const IR::P4Parser *> &redundantParsers;
bool preorder(const IR::P4Parser *parser) override;
public:
explicit FindRedundantParsers(std::set<cstring> &redundantParsers)
explicit FindRedundantParsers(std::set<const IR::P4Parser *> &redundantParsers)
: redundantParsers(redundantParsers) { }
};

/** Find .apply() calls on parsers that are on redundantParsers, and
* eliminate them.
*/
class EliminateSubparserCalls : public Transform {
const std::set<cstring> &redundantParsers;
TypeMap *typeMap;
const std::set<const IR::P4Parser *> &redundantParsers;
ReferenceMap *refMap;
const IR::Node *postorder(IR::MethodCallStatement *methodCallStmt) override;
public:
EliminateSubparserCalls(const std::set<cstring> &redundantParsers, TypeMap *typeMap)
: redundantParsers(redundantParsers), typeMap(typeMap)
EliminateSubparserCalls(const std::set<const IR::P4Parser *> &redundantParsers,
ReferenceMap *refMap)
: redundantParsers(redundantParsers), refMap(refMap)
{ }
};

class RemoveRedundantParsers : public PassManager {
std::set<cstring> redundantParsers;
std::set<const IR::P4Parser *> redundantParsers;
public:
explicit RemoveRedundantParsers(TypeMap *typeMap)
explicit RemoveRedundantParsers(ReferenceMap *refMap)
: 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)
new EliminateSubparserCalls(redundantParsers, refMap)
} {
setName("RemoveRedundantParsers");
}
Expand Down