Skip to content

Commit 694af7a

Browse files
author
Mihai Budiu
authored
Do not ignore lookup when output is not used; fixes #1768 (#1774)
* Do not ignore lookup when output is not used; fixes #1768
1 parent 20bb459 commit 694af7a

13 files changed

+358
-33
lines changed

‎backends/bmv2/common/expression.cpp

+4-6
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,11 @@ void ExpressionConverter::postorder(const IR::BoolLiteral* expression) {
107107

108108
void ExpressionConverter::postorder(const IR::MethodCallExpression* expression) {
109109
auto instance = P4::MethodInstance::resolve(expression, refMap, typeMap);
110-
if (instance->is<P4::ExternMethod>()) {
111-
auto em = instance->to<P4::ExternMethod>();
110+
if (auto em = instance->to<P4::ExternMethod>()) {
112111
if (em->originalExternType->name == corelib.packetIn.name &&
113112
em->method->name == corelib.packetIn.lookahead.name) {
114-
BUG_CHECK(expression->typeArguments->size() == 1,
115-
"Expected 1 type parameter for %1%", em->method);
113+
if (expression->typeArguments->size() != 1)
114+
::error(ErrorType::ERR_INVALID, "Expected 1 type parameter for %1%", em->method);
116115
auto targ = expression->typeArguments->at(0);
117116
auto typearg = typeMap->getTypeType(targ, true);
118117
int width = typearg->width_bits();
@@ -125,8 +124,7 @@ void ExpressionConverter::postorder(const IR::MethodCallExpression* expression)
125124
mapExpression(expression, j);
126125
return;
127126
}
128-
} else if (instance->is<P4::BuiltInMethod>()) {
129-
auto bim = instance->to<P4::BuiltInMethod>();
127+
} else if (auto bim = instance->to<P4::BuiltInMethod>()) {
130128
if (bim->name == IR::Type_Header::isValid) {
131129
auto type = typeMap->getType(bim->appliedTo, true);
132130
auto result = new Util::JsonObject();

‎backends/bmv2/common/lower.cpp

+28-2
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
#include "frontends/p4/fromv1.0/v1model.h"
1817
#include "lower.h"
18+
#include "frontends/p4/coreLibrary.h"
1919
#include "frontends/p4/methodInstance.h"
20+
#include "frontends/p4/fromv1.0/v1model.h"
2021
#include "lib/gmputil.h"
2122

2223
namespace BMV2 {
@@ -327,7 +328,7 @@ RemoveComplexExpressions::postorder(IR::MethodCallExpression* expression) {
327328
}
328329

329330
const IR::Node*
330-
RemoveComplexExpressions::postorder(IR::Statement* statement) {
331+
RemoveComplexExpressions::simpleStatement(IR::Statement* statement) {
331332
if (assignments.empty())
332333
return statement;
333334
auto block = new IR::BlockStatement(assignments);
@@ -336,4 +337,29 @@ RemoveComplexExpressions::postorder(IR::Statement* statement) {
336337
return block;
337338
}
338339

340+
const IR::Node*
341+
RemoveComplexExpressions::postorder(IR::Statement* statement) {
342+
return simpleStatement(statement);
343+
}
344+
345+
const IR::Node*
346+
RemoveComplexExpressions::postorder(IR::MethodCallStatement* statement) {
347+
auto mi = P4::MethodInstance::resolve(statement, refMap, typeMap);
348+
if (auto em = mi->to<P4::ExternMethod>()) {
349+
if (em->originalExternType->name != P4::P4CoreLibrary::instance.packetIn.name ||
350+
em->method->name != P4::P4CoreLibrary::instance.packetIn.lookahead.name)
351+
return simpleStatement(statement);
352+
auto type = em->actualMethodType->returnType;
353+
auto name = refMap->newName("tmp");
354+
LOG3("Adding variable for lookahead " << name);
355+
auto decl = new IR::Declaration_Variable(IR::ID(name), type->getP4Type());
356+
newDecls.push_back(decl);
357+
typeMap->setType(decl, typeMap->getTypeType(type, true));
358+
auto assign = new IR::AssignmentStatement(
359+
statement->srcInfo, new IR::PathExpression(name), statement->methodCall);
360+
return simpleStatement(assign);
361+
}
362+
return simpleStatement(statement);
363+
}
364+
339365
} // namespace BMV2

‎backends/bmv2/common/lower.h

+5
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ class RemoveComplexExpressionsPolicy {
6767
BMv2 does not support complex expressions for a select
6868
or as arguments to external functions.
6969
Such expressions are lifted into a temporaries.
70+
Also, convert a statement like lookahead<T>() into
71+
tmp = lookahead<T>();
7072
*/
7173
class RemoveComplexExpressions : public Transform {
7274
P4::ReferenceMap* refMap;
@@ -84,6 +86,8 @@ class RemoveComplexExpressions : public Transform {
8486
const IR::IndexedVector<IR::NamedExpression>* simplifyExpressions(
8587
const IR::IndexedVector<IR::NamedExpression>* vec);
8688

89+
const IR::Node* simpleStatement(IR::Statement* statement);
90+
8791
public:
8892
RemoveComplexExpressions(P4::ReferenceMap* refMap, P4::TypeMap* typeMap,
8993
RemoveComplexExpressionsPolicy* policy = nullptr) :
@@ -114,6 +118,7 @@ class RemoveComplexExpressions : public Transform {
114118
return control;
115119
}
116120
const IR::Node* postorder(IR::Statement* statement) override;
121+
const IR::Node* postorder(IR::MethodCallStatement* statement) override;
117122
};
118123

119124
} // namespace BMV2

‎midend/expandLookahead.cpp

+42-25
Original file line numberDiff line numberDiff line change
@@ -64,55 +64,72 @@ const IR::Expression* DoExpandLookahead::expand(
6464
}
6565
}
6666

67-
const IR::Node* DoExpandLookahead::postorder(IR::AssignmentStatement* statement) {
68-
if (!statement->right->is<IR::MethodCallExpression>())
69-
return statement;
70-
71-
auto mce = statement->right->to<IR::MethodCallExpression>();
72-
auto mi = MethodInstance::resolve(mce, refMap, typeMap);
67+
DoExpandLookahead::ExpansionInfo*
68+
DoExpandLookahead::convertLookahead(const IR::MethodCallExpression* expression) {
69+
if (expression == nullptr)
70+
return nullptr;
71+
auto mi = MethodInstance::resolve(expression, refMap, typeMap);
7372
if (!mi->is<P4::ExternMethod>())
74-
return statement;
73+
return nullptr;
7574
auto em = mi->to<P4::ExternMethod>();
7675
if (em->originalExternType->name != P4CoreLibrary::instance.packetIn.name ||
7776
em->method->name != P4CoreLibrary::instance.packetIn.lookahead.name)
78-
return statement;
77+
return nullptr;
7978

8079
// this is a call to packet_in.lookahead.
81-
BUG_CHECK(mce->typeArguments->size() == 1,
80+
BUG_CHECK(expression->typeArguments->size() == 1,
8281
"Expected 1 type parameter for %1%", em->method);
83-
auto targ = mce->typeArguments->at(0);
82+
auto targ = expression->typeArguments->at(0);
8483
auto typearg = typeMap->getTypeType(targ, true);
8584
if (!typearg->is<IR::Type_StructLike>() && !typearg->is<IR::Type_Tuple>())
86-
return statement;
85+
return nullptr;
8786

88-
int width = typearg->width_bits();
89-
if (width <= 0) {
90-
::error("%1%: type argument of %2% must be a fixed-width type",
91-
targ, P4CoreLibrary::instance.packetIn.lookahead.name);
92-
return statement;
93-
}
87+
int width = typeMap->minWidthBits(typearg, expression);
88+
if (width < 0)
89+
return nullptr;
9490

9591
auto bittype = IR::Type_Bits::get(width);
9692
auto name = refMap->newName("tmp");
9793
auto decl = new IR::Declaration_Variable(IR::ID(name), bittype, nullptr);
9894
newDecls.push_back(decl);
9995

100-
auto result = new IR::BlockStatement;
10196
auto ta = new IR::Vector<IR::Type>();
10297
ta->push_back(bittype);
103-
auto mc = new IR::MethodCallExpression(mce->srcInfo, mce->method->clone(), ta, mce->arguments);
98+
auto mc = new IR::MethodCallExpression(
99+
expression->srcInfo, expression->method->clone(), ta, expression->arguments);
104100
auto pathe = new IR::PathExpression(name);
105-
auto lookupCall = new IR::AssignmentStatement(statement->srcInfo, pathe, mc);
106-
result->push_back(lookupCall);
101+
auto lookupCall = new IR::AssignmentStatement(expression->srcInfo, pathe, mc);
102+
auto result = new ExpansionInfo;
103+
result->statement = lookupCall;
104+
result->width = (unsigned)width;
105+
result->origType = typearg;
106+
result->tmp = pathe;
107+
return result;
108+
}
109+
110+
const IR::Node* DoExpandLookahead::postorder(IR::MethodCallStatement* statement) {
111+
auto ei = convertLookahead(statement->methodCall);
112+
if (ei == nullptr)
113+
return statement;
114+
return ei->statement;
115+
}
116+
117+
const IR::Node* DoExpandLookahead::postorder(IR::AssignmentStatement* statement) {
118+
if (!statement->right->is<IR::MethodCallExpression>())
119+
return statement;
107120

108-
unsigned offset = width;
109-
expandSetValid(statement->left->clone(), typearg, &result->components);
110-
auto init = expand(pathe->clone(), typearg, &offset);
121+
auto ei = convertLookahead(statement->right->to<IR::MethodCallExpression>());
122+
if (ei == nullptr)
123+
return statement;
124+
auto result = new IR::BlockStatement;
125+
result->push_back(ei->statement);
126+
127+
expandSetValid(statement->left->clone(), ei->origType, &result->components);
128+
auto init = expand(ei->tmp->clone(), ei->origType, &ei->width);
111129
if (init == nullptr)
112130
return statement;
113131
auto assignment = new IR::AssignmentStatement(statement->srcInfo, statement->left, init);
114132
result->push_back(assignment);
115-
116133
return result;
117134
}
118135

‎midend/expandLookahead.h

+9
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,25 @@ class DoExpandLookahead : public Transform {
3434
P4::TypeMap* typeMap;
3535
IR::IndexedVector<IR::Declaration> newDecls;
3636

37+
struct ExpansionInfo {
38+
const IR::Statement* statement;
39+
unsigned width;
40+
const IR::Type* origType;
41+
const IR::PathExpression* tmp; // temporary used for result
42+
};
43+
3744
const IR::Expression* expand(
3845
const IR::PathExpression* base, const IR::Type* type, unsigned* offset);
3946
void expandSetValid(const IR::Expression* base, const IR::Type* type,
4047
IR::IndexedVector<IR::StatOrDecl>* output);
48+
ExpansionInfo* convertLookahead(const IR::MethodCallExpression* expression);
4149

4250
public:
4351
DoExpandLookahead(ReferenceMap* refMap, TypeMap* typeMap) :
4452
refMap(refMap), typeMap(typeMap) {
4553
CHECK_NULL(refMap); CHECK_NULL(typeMap); setName("DoExpandLookahead"); }
4654
const IR::Node* postorder(IR::AssignmentStatement* statement) override;
55+
const IR::Node* postorder(IR::MethodCallStatement* statement) override;
4756
const IR::Node* preorder(IR::P4Control* control) override
4857
{ prune(); return control; }
4958
const IR::Node* preorder(IR::P4Parser* parser) override

‎testdata/p4_16_samples/issue1768-bmv2.p4

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
#include <core.p4>
2+
#include <v1model.p4>
3+
4+
struct B8 {
5+
bit<(8 * 8)> bits;
6+
}
7+
8+
struct headers {}
9+
10+
struct metadata {}
11+
12+
parser MyParser(packet_in packet,
13+
out headers hdr,
14+
inout metadata meta,
15+
inout standard_metadata_t standard_metadata) {
16+
state start {
17+
packet.lookahead<B8>();
18+
transition accept;
19+
}
20+
}
21+
22+
control MyVerifyChecksum(inout headers hdr,
23+
inout metadata meta) {
24+
apply { }
25+
}
26+
27+
control MyIngress(inout headers hdr,
28+
inout metadata meta,
29+
inout standard_metadata_t standard_metadata) {
30+
apply {
31+
if (standard_metadata.parser_error != error.NoError)
32+
mark_to_drop();
33+
}
34+
}
35+
36+
control MyEgress(inout headers hdr,
37+
inout metadata meta,
38+
inout standard_metadata_t standard_metadata) {
39+
apply { }
40+
}
41+
42+
control MyComputeChecksum(inout headers hdr, inout metadata meta) {
43+
apply { }
44+
}
45+
46+
control MyDeparser(packet_out packet, in headers hdr) {
47+
apply { }
48+
}
49+
50+
V1Switch(
51+
MyParser(),
52+
MyVerifyChecksum(),
53+
MyIngress(),
54+
MyEgress(),
55+
MyComputeChecksum(),
56+
MyDeparser()
57+
) main;

‎testdata/p4_16_samples/issue1768-bmv2.stf

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
packet 0 0000
2+
packet 0 112233445566778899AABBCCDDEEFF
3+
expect 0 112233445566778899AABBCCDDEEFF$

‎testdata/p4_16_samples_outputs/issue1768-bmv2-first.p4

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
#include <core.p4>
2+
#include <v1model.p4>
3+
4+
struct B8 {
5+
bit<64> bits;
6+
}
7+
8+
struct headers {
9+
}
10+
11+
struct metadata {
12+
}
13+
14+
parser MyParser(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
15+
state start {
16+
packet.lookahead<B8>();
17+
transition accept;
18+
}
19+
}
20+
21+
control MyVerifyChecksum(inout headers hdr, inout metadata meta) {
22+
apply {
23+
}
24+
}
25+
26+
control MyIngress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
27+
apply {
28+
if (standard_metadata.parser_error != error.NoError)
29+
mark_to_drop();
30+
}
31+
}
32+
33+
control MyEgress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
34+
apply {
35+
}
36+
}
37+
38+
control MyComputeChecksum(inout headers hdr, inout metadata meta) {
39+
apply {
40+
}
41+
}
42+
43+
control MyDeparser(packet_out packet, in headers hdr) {
44+
apply {
45+
}
46+
}
47+
48+
V1Switch<headers, metadata>(MyParser(), MyVerifyChecksum(), MyIngress(), MyEgress(), MyComputeChecksum(), MyDeparser()) main;
49+

‎testdata/p4_16_samples_outputs/issue1768-bmv2-frontend.p4

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
#include <core.p4>
2+
#include <v1model.p4>
3+
4+
struct B8 {
5+
bit<64> bits;
6+
}
7+
8+
struct headers {
9+
}
10+
11+
struct metadata {
12+
}
13+
14+
parser MyParser(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
15+
state start {
16+
packet.lookahead<B8>();
17+
transition accept;
18+
}
19+
}
20+
21+
control MyVerifyChecksum(inout headers hdr, inout metadata meta) {
22+
apply {
23+
}
24+
}
25+
26+
control MyIngress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
27+
apply {
28+
if (standard_metadata.parser_error != error.NoError)
29+
mark_to_drop();
30+
}
31+
}
32+
33+
control MyEgress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
34+
apply {
35+
}
36+
}
37+
38+
control MyComputeChecksum(inout headers hdr, inout metadata meta) {
39+
apply {
40+
}
41+
}
42+
43+
control MyDeparser(packet_out packet, in headers hdr) {
44+
apply {
45+
}
46+
}
47+
48+
V1Switch<headers, metadata>(MyParser(), MyVerifyChecksum(), MyIngress(), MyEgress(), MyComputeChecksum(), MyDeparser()) main;
49+

0 commit comments

Comments
 (0)