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

Make the tree cloner also clone Member(TypeNameExpression) to ensure the result is a DAG #4539

Merged
merged 3 commits into from
Apr 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion backends/bmv2/psa_switch/psaSwitch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ void PsaSwitchBackend::convert(const IR::ToplevelBlock *tlb) {
new P4::RemoveAllUnusedDeclarations(refMap),
// Converts the DAG into a TREE (at least for expressions)
// This is important later for conversion to JSON.
new P4::ClonePathExpressions(),
new P4::CloneExpressions(),
new P4::ClearTypeMap(typeMap),
evaluator,
[this, evaluator, structure]() { toplevel = evaluator->getToplevelBlock(); },
Expand Down
2 changes: 1 addition & 1 deletion backends/bmv2/simple_switch/simpleSwitch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1186,7 +1186,7 @@ void SimpleSwitchBackend::convert(const IR::ToplevelBlock *tlb) {
new P4::FlattenLogMsg(refMap, typeMap),
// Converts the DAG into a TREE (at least for expressions)
// This is important later for conversion to JSON.
new P4::ClonePathExpressions(),
new P4::CloneExpressions(),
new P4::ClearTypeMap(typeMap),
new P4::TypeChecking(refMap, typeMap, true),
evaluator,
Expand Down
16 changes: 12 additions & 4 deletions frontends/p4/cloner.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,27 @@ limitations under the License.

namespace P4 {

/// This transform converts identical PathExpression nodes in a DAG
/// This transform converts identical PathExpression or Member nodes in a DAG
/// into distinct nodes.
class ClonePathExpressions : public Transform {
class CloneExpressions : public Transform {
public:
ClonePathExpressions() {
CloneExpressions() {
visitDagOnce = false;
setName("ClonePathExpressions");
setName("CloneExpressions");
}
const IR::Node *postorder(IR::PathExpression *path) override {
path->path = path->path->clone();
return path;
}

// Clone expressions of the form Member(TypeNameExpression)
const IR::Node *postorder(IR::Member *member) override {
if (member->expr->is<IR::TypeNameExpression>()) {
return new IR::Member(member->expr->clone(), member->member);
}
return member;
}

template <typename T>
const T *clone(const IR::Node *node) {
return node->apply(*this)->to<T>();
Expand Down
2 changes: 1 addition & 1 deletion frontends/p4/localizeActions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace P4 {

namespace {

class ParamCloner : public ClonePathExpressions {
class ParamCloner : public CloneExpressions {
public:
ParamCloner() { setName("ParamCloner"); }
const IR::Node *postorder(IR::Parameter *param) override {
Expand Down
2 changes: 1 addition & 1 deletion frontends/p4/sideEffects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ const IR::Node *DoSimplifyExpressions::preorder(IR::MethodCallExpression *mce) {

visit(mce->method);

ClonePathExpressions cloner; // a cheap version of deep copy
CloneExpressions cloner; // a cheap version of deep copy
for (auto p : *mi->substitution.getParametersInArgumentOrder()) {
auto arg = mi->substitution.lookup(p);
if (p->direction == IR::Direction::None) {
Expand Down
4 changes: 2 additions & 2 deletions frontends/p4/simplifyDefUse.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class RemoveHidden : public Transform {
};

class SimplifyDefUse : public PassManager {
class Cloner : public ClonePathExpressions {
class Cloner : public CloneExpressions {
public:
Cloner() { setName("Cloner"); }
const IR::Node *postorder(IR::EmptyStatement *stat) override {
Expand All @@ -83,7 +83,7 @@ class SimplifyDefUse : public PassManager {
LOG2("Cloning " << getOriginal()->id << " into " << result->id);
return result;
}
// Ideally we'd like ClonePathExpressions::postorder(stat),
// Ideally we'd like CloneExpressions::postorder(stat),
// but that doesn't work.
return Transform::postorder(stat);
}
Expand Down
4 changes: 2 additions & 2 deletions midend/predication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ const IR::Expression *Predication::clone(const IR::Expression *expression) {
// in the end different code will be generated for the different clones of
// an expression. This is most obvious if one clone is on the LHS and one
// on the RHS of an assigment.
ClonePathExpressions cloner;
CloneExpressions cloner;
cloner.setCalledBy(this);
return expression->apply(cloner);
}
Expand All @@ -148,7 +148,7 @@ const IR::Node *Predication::clone(const IR::AssignmentStatement *statement) {
// Expressions often need to be cloned. This is necessary because
// in the end different code will be generated for the different clones of
// an expression.
ClonePathExpressions cloner;
CloneExpressions cloner;
cloner.setCalledBy(this);
return statement->apply(cloner);
}
Expand Down
23 changes: 23 additions & 0 deletions testdata/p4_16_samples/issue4500.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
enum E {
e0
}

extern void bar(E x);

extern void baz();

void foo(E y) {
bar(y);
if (y == E.e0) baz();
}

control c() {
apply {
foo(E.e0);
}
}

control proto();
package top(proto p);

top(c()) main;
21 changes: 21 additions & 0 deletions testdata/p4_16_samples_outputs/issue4500-first.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
enum E {
e0
}

extern void bar(E x);
extern void baz();
void foo(E y) {
bar(y);
if (y == E.e0) {
baz();
}
}
control c() {
apply {
foo(E.e0);
}
}

control proto();
package top(proto p);
top(c()) main;
18 changes: 18 additions & 0 deletions testdata/p4_16_samples_outputs/issue4500-frontend.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
enum E {
e0
}

extern void bar(E x);
extern void baz();
control c() {
apply {
bar(E.e0);
if (E.e0 == E.e0) {
baz();
}
}
}

control proto();
package top(proto p);
top(c()) main;
25 changes: 25 additions & 0 deletions testdata/p4_16_samples_outputs/issue4500-midend.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
enum E {
e0
}

extern void bar(E x);
extern void baz();
control c() {
@hidden action issue4500l10() {
bar(E.e0);
baz();
}
@hidden table tbl_issue4500l10 {
actions = {
issue4500l10();
}
const default_action = issue4500l10();
}
apply {
tbl_issue4500l10.apply();
}
}

control proto();
package top(proto p);
top(c()) main;
21 changes: 21 additions & 0 deletions testdata/p4_16_samples_outputs/issue4500.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
enum E {
e0
}

extern void bar(E x);
extern void baz();
void foo(E y) {
bar(y);
if (y == E.e0) {
baz();
}
}
control c() {
apply {
foo(E.e0);
}
}

control proto();
package top(proto p);
top(c()) main;
Empty file.
Loading