Skip to content

Commit

Permalink
Fix for issue #481 (#553)
Browse files Browse the repository at this point in the history 
* Fix for issue #481

* Bugfixes

* Improved comments
  • Loading branch information
@ChrisDodd
Mihai Budiu authored and ChrisDodd committed May 4, 2017
1 parent 9f7f0fa commit 3a52ba7
Show file tree
Hide file tree
Showing 23 changed files with 1,918 additions and 346 deletions.
5 changes: 0 additions & 5 deletions backends/bmv2/Makefile.am
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,3 @@ bmv2tests.mk: $(GENTESTS) $(srcdir)/%reldir%/Makefile.am \
$(srcdir)/testdata/p4_14_samples/switch_*/switch.p4 \
$(srcdir)/testdata/p4_16_samples $(srcdir)/testdata/p4_14_samples
@$(GENTESTS) $(srcdir) bmv2 $(srcdir)/backends/bmv2/run-bmv2-test.py $^ >$@

# These are issue #481
XFAIL_TESTS += \
bmv2/testdata/p4_14_samples/09-IPv4OptionsUnparsed.p4.test \
bmv2/testdata/p4_14_samples/TLV_parsing.p4.test
16 changes: 14 additions & 2 deletions backends/bmv2/jsonconverter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2698,8 +2698,10 @@ Util::IJson* JsonConverter::convertParserStatement(const IR::StatOrDecl* stat) {
if (minst->is<P4::ExternMethod>()) {
auto extmeth = minst->to<P4::ExternMethod>();
if (extmeth->method->name.name == corelib.packetIn.extract.name) {
result->emplace("op", "extract");
if (mce->arguments->size() == 1) {
int argCount = mce->arguments->size();
if (argCount == 1 || argCount == 2) {
cstring ename = argCount == 1 ? "extract" : "extract_VL";
result->emplace("op", ename);
auto arg = mce->arguments->at(0);
auto argtype = typeMap->getType(arg, true);
if (!argtype->is<IR::Type_Header>()) {
Expand Down Expand Up @@ -2731,6 +2733,16 @@ Util::IJson* JsonConverter::convertParserStatement(const IR::StatOrDecl* stat) {
auto value = j->to<Util::JsonObject>()->get("value");
param->emplace("type", type);
param->emplace("value", value);

if (argCount == 2) {
auto arg2 = mce->arguments->at(1);
auto jexpr = conv->convert(arg2, true, false);
auto rwrap = new Util::JsonObject();
// The spec says that this must always be wrapped in an expression
rwrap->emplace("type", "expression");
rwrap->emplace("value", jexpr);
params->append(rwrap);
}
return result;
}
}
Expand Down
9 changes: 2 additions & 7 deletions backends/p4test/Makefile.am
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,6 @@ p14_to_16tests.mk: $(GENTESTS) $(srcdir)/%reldir%/Makefile.am \
@$(GENTESTS) $(srcdir) p14_to_16 $(srcdir)/backends/p4test/run-p4-sample.py $^ >$@


# First is issue #13
# Second one is issue #447
# Next ones are all issue #481
# issue #13
XFAIL_TESTS += \
p4/testdata/p4_16_samples/cast-call.p4.test \
p4/testdata/p4_16_samples/spec-ex32.p4.test \
p14_to_16/testdata/p4_14_samples/09-IPv4OptionsUnparsed.p4.test \
p14_to_16/testdata/p4_14_samples/TLV_parsing.p4.test
p4/testdata/p4_16_samples/cast-call.p4.test
1 change: 1 addition & 0 deletions frontends/p4/coreLibrary.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class PacketIn : public Model::Extern_Model {
Model::Elem lookahead;
Model::Elem advance;
Model::Elem length;
int extractSecondArgSize = 32;
};

class PacketOut : public Model::Extern_Model {
Expand Down
205 changes: 201 additions & 4 deletions frontends/p4/fromv1.0/converters.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ const IR::Node* ExpressionConverter::postorder(IR::ConcreteHeaderRef* nhr) {
return structure->conversionContext.standardMetadata->clone();
else
ref = structure->conversionContext.userMetadata->clone();
}
auto result = new IR::Member(ref, nhr->ref->name);
}
auto result = new IR::Member(nhr->srcInfo, ref, nhr->ref->name);
result->type = nhr->type;
return result;
}
Expand Down Expand Up @@ -175,7 +175,7 @@ const IR::Node* StatementConverter::preorder(IR::Apply* apply) {
auto table = structure->tables.get(apply->name);
auto newname = structure->tables.get(table);
auto tbl = new IR::PathExpression(newname);
auto method = new IR::Member(tbl, IR::ID(IR::IApply::applyMethodName));
auto method = new IR::Member(apply->srcInfo, tbl, IR::ID(IR::IApply::applyMethodName));
auto call = new IR::MethodCallExpression(apply->srcInfo, method);
if (apply->actions.size() == 0) {
auto stat = new IR::MethodCallStatement(apply->srcInfo, call);
Expand Down Expand Up @@ -516,12 +516,208 @@ class Rewriter : public Transform {
return rv;
}
};

/**
This pass uses the @length annotation set by the v1 front-end on
varbit fields and converts extracts for headers with varbit fields.
(The @length annotation is inserted as a conversion from the length
header property.) For example:
header H {
bit<8> len;
@length(len)
varbit<64> data;
}
...
H h;
pkt.extract(h);
is converted to:
header H {
bit<8> len;
varbit<64> data; // annotation removed
}
...
H h;
header H_0 {
bit<8> len;
}
header H_1 {
varbit<64> data;
}
H_0 h_0;
H_1 h_1;
pkt.extract(h_0);
pkt.extract(h_1, h_0.len);
h.setValid();
h.len = h_0.len;
h.data = h_1.data;
*/
class FixExtracts final : public Transform {
ProgramStructure* structure;

/// Newly-introduced types for each extract.
std::vector<const IR::Type_Header*> typeDecls;
/// All newly-introduced types.
// The following contains IR::Type_Header, but it is easier
// to append if the elements are Node.
IR::IndexedVector<IR::Node> allTypeDecls;
/// All newly-introduced variables, for each parser.
IR::IndexedVector<IR::Declaration> varDecls;
/// Map each newly created header with a varbit field
/// to an expression that denotes its length.
std::map<const IR::Type_Header*, const IR::Expression*> lengths;

/// If a header type contains varbit fields split it into several
/// header types, each of which starts with a varbit field. The
/// types are inserted in the typeDecls list.
void splitHeaderType(const IR::Type_Header* type) {
IR::IndexedVector<IR::StructField> fields;
const IR::Expression* length = nullptr;
for (auto f : type->fields) {
if (f->type->is<IR::Type_Varbits>()) {
cstring hname = structure->makeUniqueName(type->name);
auto htype = new IR::Type_Header(IR::ID(hname), fields);
if (length != nullptr)
lengths.emplace(htype, length);
typeDecls.push_back(htype);
fields.clear();
auto anno = f->getAnnotation(IR::Annotation::lengthAnnotation);
BUG_CHECK(anno != nullptr, "No length annotation on varbit field", f);
BUG_CHECK(anno->expr.size() == 1, "Expected exactly 1 argument", anno->expr);
length = anno->expr.at(0);
f = new IR::StructField(f->srcInfo, f->name, f->type); // lose the annotation
}
fields.push_back(f);
}
if (!typeDecls.empty() && !fields.empty()) {
cstring hname = structure->makeUniqueName(type->name);
auto htype = new IR::Type_Header(IR::ID(hname), fields);
typeDecls.push_back(htype);
lengths.emplace(htype, length);
LOG3("Split header type " << type << " into " << typeDecls.size() << " parts");
}
}

/**
This pass rewrites expressions from a @length annotation expression:
PathExpressions that refer to enclosing fields are rewritten to
refer to the proper fields in a different structure. In the example above, `len`
is translated to `h_0.len`.
*/
class RewriteLength final : public Transform {
const std::vector<const IR::Type_Header*> &typeDecls;
const IR::IndexedVector<IR::Declaration> &vars;
public:
explicit RewriteLength(const std::vector<const IR::Type_Header*> &typeDecls,
const IR::IndexedVector<IR::Declaration> &vars) :
typeDecls(typeDecls), vars(vars) { setName("RewriteLength"); }
const IR::Node* postorder(IR::PathExpression* expression) override {
if (expression->path->absolute)
return expression;
unsigned index = 0;
for (auto t : typeDecls) {
for (auto f : t->fields) {
if (f->name == expression->path->name)
return new IR::Member(
expression->srcInfo,
new IR::PathExpression(vars.at(index)->name), f->name);
}
index++;
}
return expression;
}
};

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

const IR::Node* postorder(IR::P4Program* program) override {
// P4-14 headers cannot refer to other types, so it is safe
// to prepend them to the list of declarations.
allTypeDecls.append(program->declarations);
program->declarations = allTypeDecls;
return program;
}

const IR::Node* postorder(IR::P4Parser* parser) override {
if (!varDecls.empty()) {
parser->parserLocals.append(varDecls);
varDecls.clear();
}
return parser;
}

const IR::Node* postorder(IR::MethodCallStatement* statement) override {
typeDecls.clear();
auto mce = getOriginal<IR::MethodCallStatement>()->methodCall;
LOG3("Looking up in extracts " << dbp(mce));
auto ht = ::get(structure->extractsSynthesized, mce);
if (ht == nullptr)
// not an extract
return statement;

// This is an extract method invocation
BUG_CHECK(mce->arguments->size() == 1, "%1%: expected 1 argument", mce);
auto arg = mce->arguments->at(0);

splitHeaderType(ht);
if (typeDecls.empty())
return statement;

auto result = new IR::IndexedVector<IR::StatOrDecl>();
RewriteLength rewrite(typeDecls, varDecls);
for (auto t : typeDecls) {
allTypeDecls.push_back(t);
cstring varName = structure->makeUniqueName("tmp_hdr");
auto var = new IR::Declaration_Variable(IR::ID(varName), t->to<IR::Type>());
auto length = ::get(lengths, t);
varDecls.push_back(var);
auto args = new IR::Vector<IR::Expression>();
args->push_back(new IR::PathExpression(IR::ID(varName)));
if (length != nullptr) {
length = length->apply(rewrite);
auto type = IR::Type_Bits::get(
P4::P4CoreLibrary::instance.packetIn.extractSecondArgSize);
auto cast = new IR::Cast(Util::SourceInfo(), type, length);
args->push_back(cast);
}
auto expression = new IR::MethodCallExpression(
mce->srcInfo, mce->method->clone(), args);
result->push_back(new IR::MethodCallStatement(expression));
}

auto setValid = new IR::Member(
mce->srcInfo, arg, IR::Type_Header::setValid);
result->push_back(new IR::MethodCallStatement(
new IR::MethodCallExpression(
mce->srcInfo, setValid, new IR::Vector<IR::Expression>())));
unsigned index = 0;
for (auto t : typeDecls) {
auto var = varDecls.at(index);
for (auto f : t->fields) {
auto left = new IR::Member(mce->srcInfo, arg, f->name);
auto right = new IR::Member(mce->srcInfo,
new IR::PathExpression(var->name), f->name);
auto assign = new IR::AssignmentStatement(mce->srcInfo, left, right);
result->push_back(assign);
}
index++;
}
return result;
}
};

} // namespace

///////////////////////////////////////////////////////////////

Converter::Converter() {
setStopOnError(true);
setStopOnError(true); setName("Converter");

// Discover types using P4 v1.1 type-checker
passes.emplace_back(new P4::DoConstantFolding(nullptr, nullptr));
Expand All @@ -531,6 +727,7 @@ Converter::Converter() {
passes.emplace_back(new DiscoverStructure(&structure));
passes.emplace_back(new ComputeCallGraph(&structure));
passes.emplace_back(new Rewriter(&structure));
passes.emplace_back(new FixExtracts(&structure));
}

Visitor::profile_t Converter::init_apply(const IR::Node* node) {
Expand Down
Loading

0 comments on commit 3a52ba7

Please sign in to comment.