Skip to content

Commit

Permalink
proposed fix for #1135 (#1136)
Browse files Browse the repository at this point in the history
  • Loading branch information
hanw authored Jan 15, 2018
1 parent 4c0d629 commit 319b725
Show file tree
Hide file tree
Showing 13 changed files with 350 additions and 27 deletions.
58 changes: 41 additions & 17 deletions frontends/p4/fromv1.0/converters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -590,23 +590,7 @@ class ComputeCallGraph : public Inspector {
public:
explicit ComputeCallGraph(ProgramStructure* structure) : structure(structure)
{ CHECK_NULL(structure); setName("ComputeCallGraph"); }
void postorder(const IR::Apply* apply) override {
LOG3("Scanning " << apply->name);
auto tbl = structure->tables.get(apply->name.name);
if (tbl == nullptr)
::error("Could not find table %1%", apply->name);
auto parent = findContext<IR::V1Control>();
BUG_CHECK(parent != nullptr, "%1%: Apply not within a control block?", apply);
auto ctrl = get(structure->tableMapping, tbl);
if (ctrl != nullptr && ctrl != parent) {
auto previous = get(structure->tableInvocation, tbl);
::error("Table %1% invoked from two different controls: %2% and %3%",
tbl, apply, previous);
}
LOG3("Invoking " << tbl << " in " << parent->name);
structure->tableMapping.emplace(tbl, parent);
structure->tableInvocation.emplace(tbl, apply);
}

void postorder(const IR::V1Parser* parser) override {
LOG3("Scanning parser " << parser->name);
structure->parsers.add(parser->name);
Expand Down Expand Up @@ -716,6 +700,45 @@ class ComputeCallGraph : public Inspector {
}
};

/// Table call graph should be built after the control call graph is built.
/// In the case that the program contains an unused control block, the
/// table invocation in the unused control block should not be considered.
class ComputeTableCallGraph : public Inspector {
ProgramStructure *structure;

public:
explicit ComputeTableCallGraph(ProgramStructure *structure) : structure(structure) {
CHECK_NULL(structure);
setName("ComputeTableCallGraph");
}

void postorder(const IR::Apply *apply) override {
LOG3("Scanning " << apply->name);
auto tbl = structure->tables.get(apply->name.name);
if (tbl == nullptr)
::error("Could not find table %1%", apply->name);
auto parent = findContext<IR::V1Control>();
ERROR_CHECK(parent != nullptr, "%1%: Apply not within a control block?", apply);

auto ctrl = get(structure->tableMapping, tbl);

// skip control block that is unused.
if (!structure->calledControls.isCallee(parent->name) &&
parent->name != P4V1::V1Model::instance.ingress.name &&
parent->name != P4V1::V1Model::instance.egress.name )
return;

if (ctrl != nullptr && ctrl != parent) {
auto previous = get(structure->tableInvocation, tbl);
::error("Table %1% invoked from two different controls: %2% and %3%",
tbl, apply, previous);
}
LOG3("Invoking " << tbl << " in " << parent->name);
structure->tableMapping.emplace(tbl, parent);
structure->tableInvocation.emplace(tbl, apply);
}
};

class Rewriter : public Transform {
ProgramStructure* structure;
public:
Expand Down Expand Up @@ -1059,6 +1082,7 @@ Converter::Converter() {
// Convert
passes.emplace_back(new DiscoverStructure(&structure));
passes.emplace_back(new ComputeCallGraph(&structure));
passes.emplace_back(new ComputeTableCallGraph(&structure));
passes.emplace_back(new Rewriter(&structure));
passes.emplace_back(new FixExtracts(&structure));
passes.emplace_back(new RemoveAnnotatedFields);
Expand Down
6 changes: 6 additions & 0 deletions frontends/p4/fromv1.0/programStructure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1940,6 +1940,12 @@ void ProgramStructure::createControls() {

for (auto c : controlsToDo) {
auto ct = controls.get(c);
/// do not convert control block if it is not invoked
/// by other control block and it is not ingress or egress.
if (!calledControls.isCallee(c) &&
ct != controls.get(v1model.ingress.name) &&
ct != controls.get(v1model.egress.name))
continue;
auto ctrl = convertControl(ct, controls.get(ct));
if (ctrl == nullptr)
return;
Expand Down
45 changes: 45 additions & 0 deletions testdata/p4_14_samples/double_apply.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
Copyright 2013-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.
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.
*/

header_type h {
fields {
b : 1;
}
}

metadata h m;

parser start {
return ingress;
}

action x() {}

table t {
actions { x; }
}

control c {
apply(t);
}

control e {
apply(t);
}

control ingress {
c();
}
4 changes: 2 additions & 2 deletions testdata/p4_14_samples/issue780-9.p4
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ header_type exact {
}
header exact heartlands;
parser start {
return ingress1;
return ingress;
}
action add_heartlands() {
add_header(heartlands);
}
control ingress1 { }
control ingress { }
65 changes: 65 additions & 0 deletions testdata/p4_14_samples_outputs/double_apply-first.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#include <core.p4>
#include <v1model.p4>

struct h {
bit<1> b;
}

struct metadata {
@name(".m")
h m;
}

struct headers {
}

parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
@name(".start") state start {
transition accept;
}
}

control c(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
@name(".x") action x() {
}
@name(".t") table t {
actions = {
x();
@defaultonly NoAction();
}
default_action = NoAction();
}
apply {
t.apply();
}
}

control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
@name(".c") c() c_0;
apply {
c_0.apply(hdr, meta, standard_metadata);
}
}

control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
apply {
}
}

control DeparserImpl(packet_out packet, in headers hdr) {
apply {
}
}

control verifyChecksum(inout headers hdr, inout metadata meta) {
apply {
}
}

control computeChecksum(inout headers hdr, inout metadata meta) {
apply {
}
}

V1Switch<headers, metadata>(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main;

60 changes: 60 additions & 0 deletions testdata/p4_14_samples_outputs/double_apply-frontend.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#include <core.p4>
#include <v1model.p4>

struct h {
bit<1> b;
}

struct metadata {
@name(".m")
h m;
}

struct headers {
}

parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
@name(".start") state start {
transition accept;
}
}

control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
@name("NoAction") action NoAction_0() {
}
@name(".x") action _x() {
}
@name(".t") table _t_0 {
actions = {
_x();
@defaultonly NoAction_0();
}
default_action = NoAction_0();
}
apply {
_t_0.apply();
}
}

control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
apply {
}
}

control DeparserImpl(packet_out packet, in headers hdr) {
apply {
}
}

control verifyChecksum(inout headers hdr, inout metadata meta) {
apply {
}
}

control computeChecksum(inout headers hdr, inout metadata meta) {
apply {
}
}

V1Switch<headers, metadata>(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main;

60 changes: 60 additions & 0 deletions testdata/p4_14_samples_outputs/double_apply-midend.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#include <core.p4>
#include <v1model.p4>

struct h {
bit<1> b;
}

struct metadata {
@name(".m")
h m;
}

struct headers {
}

parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
@name(".start") state start {
transition accept;
}
}

control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
@name("NoAction") action NoAction_0() {
}
@name(".x") action _x() {
}
@name(".t") table _t_0 {
actions = {
_x();
@defaultonly NoAction_0();
}
default_action = NoAction_0();
}
apply {
_t_0.apply();
}
}

control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
apply {
}
}

control DeparserImpl(packet_out packet, in headers hdr) {
apply {
}
}

control verifyChecksum(inout headers hdr, inout metadata meta) {
apply {
}
}

control computeChecksum(inout headers hdr, inout metadata meta) {
apply {
}
}

V1Switch<headers, metadata>(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main;

Loading

0 comments on commit 319b725

Please sign in to comment.