Skip to content

Commit

Permalink
proposed fix for #1135
Browse files Browse the repository at this point in the history
  • Loading branch information
hanw-bfn committed Jan 12, 2018
1 parent e737c57 commit 4f984b8
Show file tree
Hide file tree
Showing 8 changed files with 343 additions and 17 deletions.
56 changes: 39 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,43 @@ 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("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);

// skip checking control block that is unused.
if (!structure->calledControls.isCallee(parent->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 +1080,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();
}
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;

65 changes: 65 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,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_0() {
}
@name(".t") table t_0 {
actions = {
x_0();
@defaultonly NoAction();
}
default_action = NoAction();
}
apply {
t_0.apply();
}
}

control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) {
@name(".c") c() c_1;
apply {
c_1.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-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;

63 changes: 63 additions & 0 deletions testdata/p4_14_samples_outputs/double_apply.p4
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#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;
}
}
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(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main;

Empty file.

0 comments on commit 4f984b8

Please sign in to comment.