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

Fixing bug in HSIndexSimplifier path #3382

Merged
merged 2 commits into from
Jun 6, 2022
Merged

Fixing bug in HSIndexSimplifier path #3382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f73df9
fixing bug #issue3374
VolodymyrPeschanenkoIntel Jun 3, 2022
6790a69
update test
VolodymyrPeschanenkoIntel Jun 6, 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
26 changes: 13 additions & 13 deletions midend/hsIndexSimplify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const IR::Node* HSIndexTransform::postorder(IR::ArrayIndex* curArrayIndex) {
return curArrayIndex;
}

IR::Node* HSIndexSimplifier::eliminateArrayIndexes(HSIndexFinder& aiFinder,
IR::Node* HSIndexContretizer::eliminateArrayIndexes(HSIndexFinder& aiFinder,
IR::Statement* statement,
const IR::Expression* expr) {
if (aiFinder.arrayIndex == nullptr) {
Expand Down Expand Up @@ -94,7 +94,7 @@ IR::Node* HSIndexSimplifier::eliminateArrayIndexes(HSIndexFinder& aiFinder,
}
if (expr != nullptr && locals != nullptr) {
// Add case for write out of bound.
cstring typeString = expr->type->node_type_name();
cstring typeString = expr->type->toString();
const IR::PathExpression* pathExpr = nullptr;
if (generatedVariables->count(typeString) == 0) {
// Add assigment of undefined header.
Expand All @@ -120,7 +120,7 @@ IR::Node* HSIndexSimplifier::eliminateArrayIndexes(HSIndexFinder& aiFinder,
return new IR::BlockStatement(newComponents);
}

IR::Node* HSIndexSimplifier::preorder(IR::AssignmentStatement* assignmentStatement) {
IR::Node* HSIndexContretizer::preorder(IR::AssignmentStatement* assignmentStatement) {
HSIndexFinder aiFinder(locals, refMap, typeMap, generatedVariables);
assignmentStatement->left->apply(aiFinder);
if (aiFinder.arrayIndex == nullptr) {
Expand Down Expand Up @@ -157,12 +157,13 @@ class IsNonConstantArrayIndex : public KeyIsSimple, public Inspector {
}
};

IR::Node* HSIndexSimplifier::preorder(IR::P4Control* control) {
IR::Node* HSIndexContretizer::preorder(IR::P4Control* control) {
DoSimplifyKey keySimplifier(refMap, typeMap, new IsNonConstantArrayIndex());
const auto* controlKeySimplified = control->apply(keySimplifier)->to<IR::P4Control>();
auto* newControl = controlKeySimplified->clone();
IR::IndexedVector<IR::Declaration> newControlLocals;
HSIndexSimplifier hsSimplifier(refMap, typeMap, &newControlLocals, generatedVariables);
GeneratedVariablesMap blockGeneratedVariables;
HSIndexContretizer hsSimplifier(refMap, typeMap, &newControlLocals, &blockGeneratedVariables);
newControl->body = newControl->body->apply(hsSimplifier)->to<IR::BlockStatement>();
for (auto* declaration : controlKeySimplified->controlLocals) {
if (declaration->is<IR::P4Action>()) {
Expand All @@ -175,19 +176,18 @@ IR::Node* HSIndexSimplifier::preorder(IR::P4Control* control) {
return newControl;
}

IR::Node* HSIndexSimplifier::preorder(IR::P4Parser* parser) {
IR::Node* HSIndexContretizer::preorder(IR::P4Parser* parser) {
prune();
return parser;
}

IR::Node* HSIndexSimplifier::preorder(IR::BlockStatement* blockStatement) {
GeneratedVariablesMap blockGeneratedVariables;
HSIndexFinder aiFinder(locals, refMap, typeMap, &blockGeneratedVariables);
IR::Node* HSIndexContretizer::preorder(IR::BlockStatement* blockStatement) {
HSIndexFinder aiFinder(locals, refMap, typeMap, generatedVariables);
blockStatement->apply(aiFinder);
if (aiFinder.arrayIndex == nullptr) {
return blockStatement;
}
HSIndexSimplifier hsSimplifier(refMap, typeMap, locals, &blockGeneratedVariables);
HSIndexContretizer hsSimplifier(refMap, typeMap, locals, generatedVariables);
auto* newBlock = blockStatement->clone();
IR::IndexedVector<IR::StatOrDecl> newComponents;
for (auto& component : blockStatement->components) {
Expand All @@ -204,21 +204,21 @@ IR::Node* HSIndexSimplifier::preorder(IR::BlockStatement* blockStatement) {
return newBlock;
}

IR::Node* HSIndexSimplifier::preorder(IR::IfStatement* ifStatement) {
IR::Node* HSIndexContretizer::preorder(IR::IfStatement* ifStatement) {
HSIndexFinder aiFinder(locals, refMap, typeMap, generatedVariables);
ifStatement->condition->apply(aiFinder);
return eliminateArrayIndexes(aiFinder, ifStatement, nullptr);
}

IR::Node* HSIndexSimplifier::preorder(IR::MethodCallStatement* methodCallStatement) {
IR::Node* HSIndexContretizer::preorder(IR::MethodCallStatement* methodCallStatement) {
HSIndexFinder aiFinder(locals, refMap, typeMap, generatedVariables);
methodCallStatement->apply(aiFinder);
// Here we mean that in/out parameter will be replaced by correspondent assignments.
// In this case no need to consider assignment to undefined value.
return eliminateArrayIndexes(aiFinder, methodCallStatement, nullptr);
}

IR::Node* HSIndexSimplifier::preorder(IR::SwitchStatement* switchStatement) {
IR::Node* HSIndexContretizer::preorder(IR::SwitchStatement* switchStatement) {
HSIndexFinder aiFinder(locals, refMap, typeMap, generatedVariables);
switchStatement->expression->apply(aiFinder);
return eliminateArrayIndexes(aiFinder, switchStatement, nullptr);
Expand Down
19 changes: 15 additions & 4 deletions midend/hsIndexSimplify.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ typedef std::map<cstring, const IR::PathExpression*> GeneratedVariablesMap;
/// adds it to the corresponded local definitions.
class HSIndexFinder : public Inspector {
friend class HSIndexTransform;
friend class HSIndexSimplifier;
friend class HSIndexContretizer;
IR::IndexedVector<IR::Declaration>* locals;
ReferenceMap* refMap;
TypeMap* typeMap;
Expand All @@ -42,7 +42,7 @@ class HSIndexFinder : public Inspector {

/// This class substitutes index of a header stack in all occurence of found header stack.
class HSIndexTransform : public Transform {
friend class HSIndexSimplifier;
friend class HSIndexContretizer;
int index;
HSIndexFinder& hsIndexFinder;

Expand All @@ -66,14 +66,14 @@ class HSIndexTransform : public Transform {
/// hdivr0 = hdr.i;
/// if (hdivr0 == 0) { hdr.h[0] = 1;}
/// else if (hdivr0 == 1){hdr.h[1] = 1;}
class HSIndexSimplifier : public Transform {
class HSIndexContretizer : public Transform {
ReferenceMap* refMap;
TypeMap* typeMap;
IR::IndexedVector<IR::Declaration>* locals;
GeneratedVariablesMap* generatedVariables;

public:
HSIndexSimplifier(ReferenceMap* refMap, TypeMap* typeMap,
HSIndexContretizer(ReferenceMap* refMap, TypeMap* typeMap,
IR::IndexedVector<IR::Declaration>* locals = nullptr,
GeneratedVariablesMap* generatedVariables = nullptr)
: refMap(refMap), typeMap(typeMap), locals(locals), generatedVariables(generatedVariables) {
Expand All @@ -92,6 +92,17 @@ class HSIndexSimplifier : public Transform {
const IR::Expression* expr);
};

class HSIndexSimplifier : public PassManager {
public:
HSIndexSimplifier(ReferenceMap* refMap, TypeMap* typeMap) {
// remove block statements
passes.push_back(new TypeChecking(refMap, typeMap, true));
passes.push_back(new HSIndexContretizer(refMap, typeMap));
setName("HSIndexSimplifier");
}
};


} // namespace P4

#endif /* _MIDEND_HSINDEXSIMPLIFY_H_ */
166 changes: 166 additions & 0 deletions testdata/p4_16_samples/issue3374.p4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
/*
Copyright 2022 Intel Corporation

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

#include <core.p4>
#include <v1model.p4>

#define MAX_LAYERS 2
typedef bit<48> EthernetAddress;

enum bit<16> ether_type_t {
TPID = 0x8100,
IPV4 = 0x0800,
IPV6 = 0x86DD
}

header ethernet_t {
EthernetAddress dstAddr;
EthernetAddress srcAddr;
bit<16> etherType;
}

header vlan_tag_h {
bit<3> pcp;
bit<1> cfi;
bit<12> vid;
ether_type_t ether_type;
}

struct headers_t {
ethernet_t ethernet;
vlan_tag_h[MAX_LAYERS] vlan_tag;
}

struct main_metadata_t {
bit<2> depth;
bit<16> ethType;
}

parser parserImpl(
packet_in pkt,
out headers_t hdrs,
inout main_metadata_t meta,
inout standard_metadata_t stdmeta)
{
state start {
meta.depth = MAX_LAYERS - 1;
pkt.extract(hdrs.ethernet);
transition select(hdrs.ethernet.etherType) {
ether_type_t.TPID : parse_vlan_tag;
default: accept;
}
}
state parse_vlan_tag {
pkt.extract(hdrs.vlan_tag.next);
meta.depth = meta.depth - 1;
transition select(hdrs.vlan_tag.last.ether_type) {
ether_type_t.TPID : parse_vlan_tag;
default: accept;
}
}
}

control verifyChecksum(
inout headers_t hdr,
inout main_metadata_t meta)
{
apply { }
}

control ingressImpl(
inout headers_t hdrs,
inout main_metadata_t meta,
inout standard_metadata_t stdmeta)
{
action drop_packet() {
mark_to_drop(stdmeta);
}
action execute() {
meta.ethType = hdrs.vlan_tag[meta.depth - 1].ether_type;
hdrs.vlan_tag[meta.depth - 1].ether_type = (ether_type_t)16w2;
hdrs.vlan_tag[meta.depth].vid = (bit<12>)hdrs.vlan_tag[meta.depth].cfi;
hdrs.vlan_tag[meta.depth].vid = hdrs.vlan_tag[meta.depth-1].vid;
// hdrs.vlan_tag[meta.depth].vid = hdrs.vlan_tag[1].vid;
}
action execute_1() {
drop_packet();
}

table stub {
key = {
hdrs.vlan_tag[meta.depth].vid : exact;
}

actions = {
execute;
}
const default_action = execute;
size=1000000;
}

table stub1 {
key = {
hdrs.ethernet.etherType : exact;
}

actions = {
execute_1;
}
const default_action = execute_1;
size=1000000;
}
apply {
switch (hdrs.vlan_tag[meta.depth].vid) {
// switch (hdrs.vlan_tag[0].vid) {
12w1: { stub.apply();}
12w2: {
if (hdrs.vlan_tag[meta.depth].ether_type == hdrs.ethernet.etherType)
stub1.apply();
}
}
}
}

control egressImpl(
inout headers_t hdr,
inout main_metadata_t meta,
inout standard_metadata_t stdmeta)
{
apply { }
}

control updateChecksum(
inout headers_t hdr,
inout main_metadata_t meta)
{
apply { }
}

control deparserImpl(
packet_out pkt,
in headers_t hdr)
{
apply {
pkt.emit(hdr.ethernet);
}
}

V1Switch(parserImpl(),
verifyChecksum(),
ingressImpl(),
egressImpl(),
updateChecksum(),
deparserImpl()) main;
9 changes: 4 additions & 5 deletions testdata/p4_16_samples_outputs/control-hs-index-test6-midend.p4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ parser p(packet_in pkt, out headers hdr, inout Meta m, inout standard_metadata_t

control ingress(inout headers h, inout Meta m, inout standard_metadata_t sm) {
bit<32> hsiVar;
bit<32> hsiVar_0;
bit<32> hsVar;
@noWarn("unused") @name(".NoAction") action NoAction_1() {
}
Expand All @@ -66,7 +65,7 @@ control ingress(inout headers h, inout Meta m, inout standard_metadata_t sm) {
key_0 = hsVar;
}
@hidden action controlhsindextest6l48_2() {
hsiVar_0 = h.i.index;
hsiVar = h.i.index;
}
@hidden table tbl_controlhsindextest6l48 {
actions = {
Expand Down Expand Up @@ -94,11 +93,11 @@ control ingress(inout headers h, inout Meta m, inout standard_metadata_t sm) {
}
apply {
tbl_controlhsindextest6l48.apply();
if (hsiVar_0 == 32w0) {
if (hsiVar == 32w0) {
tbl_controlhsindextest6l48_0.apply();
} else if (hsiVar_0 == 32w1) {
} else if (hsiVar == 32w1) {
tbl_controlhsindextest6l48_1.apply();
} else if (hsiVar_0 >= 32w1) {
} else if (hsiVar >= 32w1) {
tbl_controlhsindextest6l48_2.apply();
}
t_0.apply();
Expand Down
Loading