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

Shorten the Identifer name, including dots(.) in Member expression #3175

Merged
merged 1 commit into from
Apr 8, 2022
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
70 changes: 69 additions & 1 deletion BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ filegroup(
srcs = [
"frontends/p4-14/ir-v1.def",
"backends/bmv2/bmv2.def",
"backends/dpdk/dpdk.def",
# p4c extensions may set this target to a `filegroup` containing
# additional .def files.
"@com_github_p4lang_p4c_extension//:ir_extensions",
Expand Down Expand Up @@ -190,13 +191,17 @@ cc_library(
"frontends/parsers/p4/p4lexer.cc",
"frontends/parsers/v1/v1lexer.cc",
"ir/ir-generated.cpp",
"backends/dpdk/spec.cpp",
"backends/dpdk/dbprint-dpdk.cpp",
"backends/dpdk/printUtils.cpp",
],
textual_hdrs = glob([
"ir/**/*.h",
"frontends/**/*.h",
"frontends/**/*.hpp",
"midend/**/*.h",
"control-plane/**/*.h",
"backends/dpdk/*.h",
]) + [
":p4_parser_yacc",
":v1_parser_yacc",
Expand Down Expand Up @@ -275,8 +280,71 @@ cc_binary(
data = [":p4include"],
)

# This builds the p4test backend.
# dpdk backend
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps for the bazel build you want to request a review from @smolkaj

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unable to add @smolkaj to reviewer list. may be due to permission issues. Can you please add him?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That mention is enough, he had been notified. If he doesn't reply in a couple of days I will merge it anyway

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, taking a look :)

genrule(
name = "dest_dir_dpdk",
srcs = ["backends/dpdk/control-plane/proto/p4info.proto"],
outs = ["p4/config/dpdk/p4info.proto"],
cmd = "mkdir -p p4/config/dpdk; cp $(SRCS) $(OUTS)",
visibility = ["//visibility:private"],
)

proto_library(
name = "p4info_dpdk_proto",
srcs = ["p4/config/dpdk/p4info.proto"],
deps = [
"@com_google_protobuf//:any_proto",
"@com_google_protobuf//:descriptor_proto",
],
)

cc_proto_library(
name = "p4info_dpdk_cc_proto",
deps = [":p4info_dpdk_proto"],
)

cc_library(
name = "p4c_dpdk_lib",
srcs = glob(
["backends/dpdk/*.cpp", "backends/dpdk/control-plane/*.cpp", "backends/bmv2/common/lower.cpp"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: maybe put each file on a separate line for consistency / readability?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and below

exclude = ["backends/dpdk/main.cpp", "backends/dpdk/spec.cpp", "backends/dpdk/printUtils.cpp", "backends/dpdk/dbprint-dpdk.cpp"],
),
hdrs = glob(["backends/dpdk/*.h", "backends/dpdk/control-plane/*.h", "backends/bmv2/common/lower.h"]),
deps = [
":ir_frontend_midend_control_plane",
":lib",
":p4info_dpdk_cc_proto",
],
)

genrule(
name = "p4c_dpdk_version",
srcs = ["backends/dpdk/version.h.cmake"],
outs = ["backends/dpdk/version.h"],
cmd = "sed 's|@P4C_VERSION@|0.0.0.0|g' $(SRCS) > $(OUTS)",
visibility = ["//visibility:private"],
)

cc_binary(
name = "p4c_dpdk",
srcs = [
"backends/dpdk/main.cpp",
"backends/dpdk/version.h",
],
linkopts = [
"-lgmp",
"-lgmpxx",
],
deps = [
":p4c_dpdk_lib",
":ir_frontend_midend_control_plane",
":lib",
],
data = [":p4include"],
)


# This builds the p4test backend.
cc_binary(
name = "p4c_backend_p4test",
srcs = [
Expand Down
4 changes: 2 additions & 2 deletions backends/dpdk/backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ void DpdkBackend::convert(const IR::ToplevelBlock *tlb) {
new DpdkAsmOptimization,
new CollectUsedMetadataField(used_fields),
new RemoveUnusedMetadataFields(used_fields),
new ValidateTableKeys()
new ValidateTableKeys(),
new ShortenTokenLength(),
};

dpdk_program = dpdk_program->apply(post_code_gen)->to<IR::DpdkAsmProgram>();
Expand All @@ -136,5 +137,4 @@ void DpdkBackend::convert(const IR::ToplevelBlock *tlb) {
void DpdkBackend::codegen(std::ostream &out) const {
dpdk_program->toSpec(out) << std::endl;
}

} // namespace DPDK
2 changes: 1 addition & 1 deletion backends/dpdk/dbprint-dpdk.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include <ir/ir.h>
#include "ir/ir.h"

void IR::DpdkJmpLabelStatement::dbprint(std::ostream& out) const {
out << "jmp " << label << std::endl;
Expand Down
4 changes: 2 additions & 2 deletions backends/dpdk/dpdk.def
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ class DpdkTable {

class DpdkSelector {
inline cstring name;
inline cstring group_id;
inline cstring member_id;
Expression group_id;
Expression member_id;
Key selectors;
int n_groups_max;
int n_members_per_group_max;
Expand Down
1 change: 0 additions & 1 deletion backends/dpdk/dpdkArch.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
#ifndef BACKENDS_DPDK_DPDKARCH_H_
#define BACKENDS_DPDK_DPDKARCH_H_

#include <ir/ir.h>
#include "frontends/common/resolveReferences/resolveReferences.h"
#include "frontends/p4/evaluator/evaluator.h"
#include "frontends/p4/typeMap.h"
Expand Down
2 changes: 2 additions & 0 deletions backends/dpdk/dpdkAsmOpt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,4 +279,6 @@ bool ValidateTableKeys::preorder(const IR::DpdkAsmProgram *p) {
}
return false;
}

size_t ShortenTokenLength::count = 0;
} // namespace DPDK
118 changes: 118 additions & 0 deletions backends/dpdk/dpdkAsmOpt.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,124 @@ class ValidateTableKeys : public Inspector {
int getFieldSizeBits(const IR::Type *field_type);
};

// This pass shorten the Identifier length
class ShortenTokenLength : public Transform {
ordered_map<cstring, cstring> newNameMap;
static size_t count;
// Currently Dpdk allows Identifier of 63 char long or less
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just a silly restriction, since I suspect the identifiers do not have a runtime representation.
wouldn't it be easier to fix this in the DPDK spec/implementation?
we're no longer in the 1970s when every byte counted.

// including dots(.) for member exp.
// worst case member expression will look like below(for headers)
// 1.30.30 => 63(including dot(.))
// if id name less than allowedLength keep it same
cstring shortenString(cstring str, size_t allowedLength = 60) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a class which does this, MinimalNameGenerator.
You should reuse that code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This runs post codegen and applied on some dpdk specific types too.

if (str.size() <= allowedLength)
return str;
auto itr = newNameMap.find(str);
if (itr != newNameMap.end())
return itr->second;
// make sure new string length less or equal allowedLength
cstring newStr = str.substr(0, allowedLength - std::to_string(count).size());
newStr += std::to_string(count);
count++;
newNameMap.insert(std::pair<cstring, cstring>(str, newStr));
origNameMap.insert(std::pair<cstring, cstring>(newStr, str));
return newStr;
}

public:
static ordered_map<cstring, cstring> origNameMap;

const IR::Node* preorder(IR::Member *m) override {
if (m->toString().startsWith("m."))
m->member = shortenString(m->member);
else
m->member = shortenString(m->member, 30);
return m;
}

const IR::Node* preorder(IR::DpdkStructType *s) override {
if (s->getAnnotations()->getSingle("__packet_data__")) {
s->name = shortenString(s->name);
IR::IndexedVector<IR::StructField> changedFields;
for (auto field : s->fields) {
IR::StructField *f = new IR::StructField(field->name, field->type);
f->name = shortenString(f->name, 30);
changedFields.push_back(f);
}
return new IR::DpdkStructType(s->srcInfo, s->name,
s->annotations, changedFields);
} else {
s->name = shortenString(s->name);
IR::IndexedVector<IR::StructField> changedFields;
for (auto field : s->fields) {
IR::StructField *f = new IR::StructField(field->name, field->type);
f->name = shortenString(f->name);
changedFields.push_back(f);
}
return new IR::DpdkStructType(s->srcInfo, s->name,
s->annotations, changedFields);
}
return s;
}

const IR::Node* preorder(IR::DpdkHeaderType *h) override {
h->name = shortenString(h->name, 30);
IR::IndexedVector<IR::StructField> changedFields;
for (auto field : h->fields) {
IR::StructField *f = new IR::StructField(field->name, field->type);
f->name = shortenString(f->name, 30);
changedFields.push_back(f);
}
return new IR::DpdkHeaderType(h->srcInfo, h->name,
h->annotations, changedFields);
}

const IR::Node* preorder(IR::DpdkExternDeclaration *e) override {
e->name = shortenString(e->name);
return e;
}

const IR::Node* preorder(IR::Declaration *g) override {
g->name = shortenString(g->name);
return g;
}

const IR::Node* preorder(IR::Path *p) override {
p->name = shortenString(p->name);
return p;
}

const IR::Node* preorder(IR::DpdkAction *a) override {
a->name = shortenString(a->name);
return a;
}

const IR::Node* preorder(IR::DpdkTable *t) override {
t->name = shortenString(t->name);
return t;
}

const IR::Node* preorder(IR::DpdkLearner *l) override {
l->name = shortenString(l->name);
return l;
}

const IR::Node* preorder(IR::DpdkSelector *s) override {
s->name = shortenString(s->name);
return s;
}

const IR::Node* preorder(IR::DpdkLearnStatement *ls) override{
ls->action = shortenString(ls->action);
return ls;
}

const IR::Node* preorder(IR::DpdkApplyStatement *as) override{
as->table = shortenString(as->table);
return as;
}
};

// Instructions can only appear in actions and apply block of .spec file.
// All these individual passes work on the actions and apply block of .spec file.
class DpdkAsmOptimization : public PassRepeated {
Expand Down
11 changes: 6 additions & 5 deletions backends/dpdk/dpdkProgram.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,8 @@ bool ConvertToDpdkControl::checkTableValid(const IR::P4Table *a) {
return true;
}

boost::optional<cstring> ConvertToDpdkControl::getIdFromProperty(const IR::P4Table* table,
boost::optional<const IR::Member*>
ConvertToDpdkControl::getMemExprFromProperty(const IR::P4Table* table,
cstring propertyName) {
auto property = table->properties->getProperty(propertyName);
if (property == nullptr) return boost::none;
Expand All @@ -577,7 +578,7 @@ boost::optional<cstring> ConvertToDpdkControl::getIdFromProperty(const IR::P4Tab
return boost::none;
}

return expr->to<IR::Member>()->toString();
return expr->to<IR::Member>();
}

boost::optional<int> ConvertToDpdkControl::getNumberFromProperty(const IR::P4Table* table,
Expand Down Expand Up @@ -607,8 +608,8 @@ bool ConvertToDpdkControl::preorder(const IR::P4Table *t) {
return false;

if (t->properties->getProperty("selector") != nullptr) {
auto group_id = getIdFromProperty(t, "group_id");
auto member_id = getIdFromProperty(t, "member_id");
auto group_id = getMemExprFromProperty(t, "group_id");
auto member_id = getMemExprFromProperty(t, "member_id");
auto selector_key = t->properties->getProperty("selector");
auto n_groups_max = getNumberFromProperty(t, "n_groups_max");
auto n_members_per_group_max = getNumberFromProperty(t, "n_members_per_group_max");
Expand All @@ -618,7 +619,7 @@ bool ConvertToDpdkControl::preorder(const IR::P4Table *t) {
return false;

auto selector = new IR::DpdkSelector(t->name,
*group_id, *member_id, selector_key->value->to<IR::Key>(),
(*group_id)->clone(), (*member_id)->clone(), selector_key->value->to<IR::Key>(),
*n_groups_max, *n_members_per_group_max);

selectors.push_back(selector);
Expand Down
2 changes: 1 addition & 1 deletion backends/dpdk/dpdkProgram.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class ConvertToDpdkControl : public Inspector {
void add_table(const IR::DpdkLearner*s) { learners.push_back(s); }
void add_action(const IR::DpdkAction *a) { actions.push_back(a); }

boost::optional<cstring> getIdFromProperty(const IR::P4Table*, cstring);
boost::optional<const IR::Member*> getMemExprFromProperty(const IR::P4Table*, cstring);
boost::optional<int> getNumberFromProperty(const IR::P4Table*, cstring);
};

Expand Down
51 changes: 50 additions & 1 deletion backends/dpdk/run-dpdk-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@

# Runs the compiler on a sample P4 V1.2 program


from os import environ
from threading import Timer
from subprocess import Popen,PIPE
from threading import Thread
import errno
Expand Down Expand Up @@ -160,6 +161,54 @@ def check_generated_files(options, tmpdir, expecteddir):
return SUCCESS
if result != SUCCESS and not ignoreStderr(options):
return result
if produced.endswith(".spec") and environ.get('DPDK_PIPELINE') is not None:
clifile = os.path.splitext(produced)[0] + ".cli"
with open(clifile,'w',encoding = 'utf-8') as f:
f.write("; SPDX-License-Identifier: BSD-3-Clause\n")
f.write("; Copyright(c) 2020 Intel Corporation\n")
f.write("\n")
f.write("mempool MEMPOOL0 buffer 9304 pool 32K cache 256 cpu 0\n")
f.write("\n")
f.write("link LINK0 dev 0000:00:04.0 rxq 1 128 MEMPOOL0 txq 1 512 promiscuous on\n")
f.write("link LINK1 dev 0000:00:05.0 rxq 1 128 MEMPOOL0 txq 1 512 promiscuous on\n")
f.write("link LINK2 dev 0000:00:06.0 rxq 1 128 MEMPOOL0 txq 1 512 promiscuous on\n")
f.write("link LINK3 dev 0000:00:07.0 rxq 1 128 MEMPOOL0 txq 1 512 promiscuous on\n")
f.write("\n")
f.write("pipeline PIPELINE0 create 0\n")
f.write("\n")
f.write("pipeline PIPELINE0 port in 0 link LINK0 rxq 0 bsz 32\n")
f.write("pipeline PIPELINE0 port in 1 link LINK1 rxq 0 bsz 32\n")
f.write("pipeline PIPELINE0 port in 2 link LINK2 rxq 0 bsz 32\n")
f.write("pipeline PIPELINE0 port in 3 link LINK3 rxq 0 bsz 32\n")
f.write("\n")
f.write("pipeline PIPELINE0 port out 0 link LINK0 txq 0 bsz 32\n")
f.write("pipeline PIPELINE0 port out 1 link LINK1 txq 0 bsz 32\n")
f.write("pipeline PIPELINE0 port out 2 link LINK2 txq 0 bsz 32\n")
f.write("pipeline PIPELINE0 port out 3 link LINK3 txq 0 bsz 32\n")
f.write("\n")
f.write("pipeline PIPELINE0 build ")
f.write(str(expected))
f.write("\n")
f.write("pipeline PIPELINE0 commit\n")
f.write("thread 1 pipeline PIPELINE0 enable\n")
f.write("pipeline PIPELINE0 abort\n")
f.close()
dpdk_log = os.path.splitext(produced)[0] + ".log"
def kill(process):
process.kill()
print("exec " + environ.get('DPDK_PIPELINE')+ " -n 4 -c 0x3 -- -s " + str(clifile)+ " 1>" + dpdk_log)
pipe = subprocess.Popen("exec " + environ.get('DPDK_PIPELINE')+ " -n 4 -c 0x3 -- -s " + str(clifile)+ " 1>" + dpdk_log ,cwd=".", shell=True)
timer = Timer(5, kill, [pipe])
try:
timer.start()
out, err = pipe.communicate()
finally:
timer.cancel()
with open(dpdk_log, "r") as f:
readfile = f.read()
if 'Error' in readfile:
print(readfile)
return FAILURE
return SUCCESS

def file_name(tmpfolder, base, suffix, ext):
Expand Down
Loading