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

Fix for issue #133 #135

Closed
wants to merge 8 commits into from
Closed
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
9 changes: 6 additions & 3 deletions backends/bmv2/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,18 @@ p4c_bm2_ss_SOURCES = \
backends/bmv2/bmv2.cpp \
backends/bmv2/analyzer.h \
backends/bmv2/analyzer.cpp \
backends/bmv2/copyStructures.h \
backends/bmv2/copyStructures.cpp \
backends/bmv2/jsonconverter.h \
backends/bmv2/jsonconverter.cpp \
backends/bmv2/inlining.h \
backends/bmv2/inlining.cpp \
backends/bmv2/midend.h \
backends/bmv2/midend.cpp \
backends/bmv2/lower.h \
backends/bmv2/lower.cpp
backends/bmv2/lower.cpp \
backends/bmv2/validateProperties.h \
backends/bmv2/validateProperties.cpp

cpplint_FILES += $(p4c_bm2_ss_SOURCES)

Expand All @@ -49,8 +53,7 @@ bmv2tests.mk: $(GENTESTS) $(srcdir)/%reldir%/Makefile.am \
$(srcdir)/testdata/p4_16_samples $(srcdir)/testdata/p4_14_samples
@$(GENTESTS) $(srcdir) bmv2 $(srcdir)/backends/bmv2/run-bmv2-test.py $^ >$@

# First 3 exhibit P4_14 features still unimplemented
# Last two are back-end bugs
# These exhibit P4_14 features still unimplemented
XFAIL_TESTS += \
bmv2/testdata/p4_14_samples/09-IPv4OptionsUnparsed.p4.test \
bmv2/testdata/p4_14_samples/TLV_parsing.p4.test \
Expand Down
52 changes: 52 additions & 0 deletions backends/bmv2/copyStructures.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
Copyright 2016 VMware, 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.
*/

#include "copyStructures.h"

namespace BMV2 {

const IR::Node* DoCopyStructures::postorder(IR::AssignmentStatement* statement) {
auto ltype = typeMap->getType(statement->left, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be a preorder so that assignments of nested structs will get flattened properly too? However, that would likely require fixing the code to use and maintain the types in the expression nodes properly, rather than the typeMap, which will be out of date after the first flattening.

if (!ltype->is<IR::Type_StructLike>())
return statement;

auto retval = new IR::Vector<IR::StatOrDecl>();
auto strct = ltype->to<IR::Type_StructLike>();
if (statement->right->is<IR::ListExpression>()) {
auto list = statement->right->to<IR::ListExpression>();
unsigned index = 0;
for (auto f : *strct->fields) {
auto right = list->components->at(index);
auto left = new IR::Member(Util::SourceInfo(), statement->left, f->name);
retval->push_back(new IR::AssignmentStatement(statement->srcInfo, left, right));
index++;
}
} else {
if (ltype->is<IR::Type_Header>())
// Leave headers as they are -- copy_header will also copy the valid bit
return statement;

for (auto f : *strct->fields) {
auto right = new IR::Member(Util::SourceInfo(), statement->right, f->name);
auto left = new IR::Member(Util::SourceInfo(), statement->left, f->name);
retval->push_back(new IR::AssignmentStatement(statement->srcInfo, left, right));
}
}

return retval;
}

} // namespace BMV2
46 changes: 46 additions & 0 deletions backends/bmv2/copyStructures.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
Copyright 2016 VMware, 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.
*/

#ifndef _BACKENDS_BMV2_COPYSTRUCTURES_H_
#define _BACKENDS_BMV2_COPYSTRUCTURES_H_

#include "ir/ir.h"
#include "frontends/p4/typeChecking/typeChecker.h"

namespace BMV2 {

// Convert assignments between structures to assignments between fields
class DoCopyStructures : public Transform {
P4::TypeMap* typeMap;
public:
explicit DoCopyStructures(P4::TypeMap* typeMap) : typeMap(typeMap)
{ CHECK_NULL(typeMap); setName("DoCopyStructures"); }
const IR::Node* postorder(IR::AssignmentStatement* statement) override;
};

class CopyStructures : public PassRepeated {
public:
CopyStructures(P4::ReferenceMap* refMap, P4::TypeMap* typeMap) :
PassManager({}) {
CHECK_NULL(refMap); CHECK_NULL(typeMap); setName("CopyStructures");
passes.emplace_back(new P4::TypeChecking(refMap, typeMap));
passes.emplace_back(new DoCopyStructures(typeMap));
}
};

} // namespace BMV2

#endif /* _BACKENDS_BMV2_COPYSTRUCTURES_H__ */
31 changes: 25 additions & 6 deletions backends/bmv2/jsonconverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ class ExpressionConverter : public Inspector {
if (param == converter->stdMetadataParameter) {
result->emplace("type", "field");
auto e = mkArrayField(result, "value");
e->append("standard_metadata");
e->append(converter->jsonMetadataParameterName);
e->append(expression->member);
} else {
if (type->is<IR::Type_Stack>()) {
Expand Down Expand Up @@ -404,11 +404,29 @@ class ExpressionConverter : public Inspector {
CHECK_NULL(l);
result->emplace("type", "field");
auto e = mkArrayField(result, "value");
if (l->is<Util::JsonObject>())
e->append(l->to<Util::JsonObject>()->get("value"));
else
if (l->is<Util::JsonObject>()) {
auto lv = l->to<Util::JsonObject>()->get("value");
if (lv->is<Util::JsonArray>()) {
// nested struct reference [ ["m", "f"], "x" ] => [ "m", "f.x" ]
auto array = lv->to<Util::JsonArray>();
BUG_CHECK(array->size() == 2, "expected 2 elements");
auto first = array->at(0);
auto second = array->at(1);
BUG_CHECK(second->is<Util::JsonValue>(), "expected a value");
e->append(first);
cstring nestedField = second->to<Util::JsonValue>()->getString();
nestedField += "." + expression->member.name;
e->append(nestedField);
} else if (lv->is<Util::JsonValue>()) {
e->append(lv);
e->append(expression->member.name);
} else {
BUG("%1%: Unexpected json", lv);
}
} else {
e->append(l);
e->append(expression->member.name);
e->append(expression->member.name);
}
}
}
map.emplace(expression, result);
Expand Down Expand Up @@ -531,9 +549,10 @@ class ExpressionConverter : public Inspector {
auto decl = converter->refMap->getDeclaration(expression->path, true);
if (auto param = decl->to<IR::Parameter>()) {
if (param == converter->stdMetadataParameter) {
// This is a flat struct
auto result = new Util::JsonObject();
result->emplace("type", "header");
result->emplace("value", "standard_metadata");
result->emplace("value", converter->jsonMetadataParameterName);
map.emplace(expression, result);
return;
}
Expand Down
3 changes: 2 additions & 1 deletion backends/bmv2/jsonconverter.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2013-present Barefoot Networks, Inc.
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.
Expand Down Expand Up @@ -69,6 +69,7 @@ class JsonConverter final {
const IR::Parameter* headerParameter;
const IR::Parameter* userMetadataParameter;
const IR::Parameter* stdMetadataParameter;
cstring jsonMetadataParameterName = "standard_metadata";

private:
Util::JsonArray *headerTypes;
Expand Down
7 changes: 6 additions & 1 deletion backends/bmv2/midend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ limitations under the License.
*/

#include "midend.h"
#include "validateProperties.h"
#include "lower.h"
#include "inlining.h"
#include "copyStructures.h"
#include "midend/actionsInlining.h"
#include "midend/removeReturns.h"
#include "midend/moveConstructors.h"
Expand Down Expand Up @@ -117,7 +119,7 @@ void MidEnd::setup_for_P4_16(CompilerOptions&) {
new P4::SimplifyControlFlow(&refMap, &typeMap),
new P4::SynthesizeActions(&refMap, &typeMap),
new P4::MoveActionsToTables(&refMap, &typeMap),
});
});
}


Expand All @@ -134,11 +136,14 @@ MidEnd::MidEnd(CompilerOptions& options) {
// BMv2-specific passes
auto evaluator = new P4::EvaluatorPass(&refMap, &typeMap);
addPasses({
// TODO: replace Tuple types with structs
new P4::SimplifyControlFlow(&refMap, &typeMap),
new P4::TypeChecking(&refMap, &typeMap),
new P4::RemoveLeftSlices(&typeMap),
new P4::TypeChecking(&refMap, &typeMap),
new LowerExpressions(&typeMap),
new CopyStructures(&refMap, &typeMap),
new ValidateTableProperties(),
new P4::ConstantFolding(&refMap, &typeMap),
evaluator,
new VisitFunctor([this, evaluator]() { toplevel = evaluator->getToplevelBlock(); })
Expand Down
19 changes: 19 additions & 0 deletions backends/bmv2/validateProperties.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#include "validateProperties.h"

namespace BMV2 {

void ValidateTableProperties::postorder(const IR::TableProperty* property) {
static cstring knownProperties[] = {
"actions",
"default_action",
"key",
"implementation",
"size"
};
for (size_t i=0; i < ELEMENTS(knownProperties); i++)
if (property->name.name == knownProperties[i])
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer for (auto propName : knownProperties) for this kind of loop. Or even something like

static std::set<cstring> knownProperties = { ... };
if (!knownProperties.count(property->name))
    warning(...

return;
::warning("Unknown table property: %1%", property);
}

} // namespace BMV2
35 changes: 35 additions & 0 deletions backends/bmv2/validateProperties.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
Copyright 2016 VMware, 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.
*/

#ifndef _BACKENDS_BMV2_VALIDATEPROPERTIES_H_
#define _BACKENDS_BMV2_VALIDATEPROPERTIES_H_

#include "ir/ir.h"
#include "frontends/p4/typeMap.h"

namespace BMV2 {

// Checks to see if there are any unknown properties.
class ValidateTableProperties : public Inspector {
public:
ValidateTableProperties()
{ setName("ValidateTableProperties"); }
void postorder(const IR::TableProperty* property) override;
};

} // namespace BMV2

#endif /* _BACKENDS_BMV2_VALIDATEPROPERTIES_H_ */
4 changes: 2 additions & 2 deletions frontends/common/resolveReferences/resolveReferences.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ ResolutionContext::resolve(IR::ID name, P4::ResolutionType type, bool previousOn

auto vector = decls->toVector();
if (!vector->empty()) {
LOG2("Resolved in " << current->getNode());
LOG2("Resolved in " << dbp(current->getNode()));
return vector;
} else {
continue;
Expand Down Expand Up @@ -105,7 +105,7 @@ ResolutionContext::resolve(IR::ID name, P4::ResolutionType type, bool previousOn
continue;
}

LOG2("Resolved in " << current->getNode());
LOG2("Resolved in " << dbp(current->getNode()));
auto result = new std::vector<const IR::IDeclaration*>();
result->push_back(decl);
return result;
Expand Down
2 changes: 1 addition & 1 deletion frontends/p4/fromv1.0/v1model.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ struct Parser_Model : public ::Model::Elem {
packetParam("packet", P4::P4CoreLibrary::instance.packetIn, 0),
headersParam("hdr", headersType, 1),
metadataParam("meta", userMetaType, 2),
standardMetadataParam("stdMeta", standardMetadataType, 3)
standardMetadataParam("standard_metadata", standardMetadataType, 3)
{}
::Model::Param_Model packetParam;
::Model::Param_Model headersParam;
Expand Down
35 changes: 23 additions & 12 deletions frontends/p4/sideEffects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,28 @@ class DismantleExpression : public Transform {
typeMap->setType(result->final, type);
if (leftValue)
typeMap->setLeftValue(result->final);

if (TableApplySolver::isHit(expression, refMap, typeMap)) {
auto ctx = getContext();
// f(t.apply().hit) =>
// bool tmp;
// if (t.apply().hit)
// tmp = true;
// else
// tmp = false;
// f(tmp)
BUG_CHECK(type->is<IR::Type_Boolean>(), "%1%: not boolean", type);
if (ctx != nullptr && ctx->node->is<IR::Expression>()) {
auto tmp = result->createTemporary(type);
auto path = new IR::PathExpression(IR::ID(tmp));
auto tstat = new IR::AssignmentStatement(Util::SourceInfo(), path->clone(), new IR::BoolLiteral(true));
auto fstat = new IR::AssignmentStatement(Util::SourceInfo(), path->clone(), new IR::BoolLiteral(false));
auto ifStatement = new IR::IfStatement(Util::SourceInfo(), result->final, tstat, fstat);
result->statements->push_back(ifStatement);
result->final = path->clone();
typeMap->setType(result->final, type);
}
}
prune();
return result->final;
}
Expand Down Expand Up @@ -245,20 +267,9 @@ class DismantleExpression : public Transform {
const IR::Node* preorder(IR::LAnd* expression) override { return shortCircuit(expression); }
const IR::Node* preorder(IR::LOr* expression) override { return shortCircuit(expression); }

const IR::Node* postorder(IR::Member* expression) override {
LOG1("Visiting " << dbp(expression));
auto orig = getOriginal<IR::Member>();
result->final = expression;
if (typeMap->isLeftValue(orig))
typeMap->setLeftValue(result->final);
if (typeMap->isCompileTimeConstant(orig))
typeMap->setCompileTimeConstant(orig);
return result->final;
}

const IR::Node* preorder(IR::MethodCallExpression* mce) override {
BUG_CHECK(!leftValue, "%1%: method on left hand side?", mce);
LOG1("Visiting " << mce);
LOG1("Visiting " << dbp(mce));
auto orig = getOriginal<IR::MethodCallExpression>();
auto type = typeMap->getType(orig, true);
if (!SideEffects::check(orig, refMap, typeMap)) {
Expand Down
5 changes: 4 additions & 1 deletion frontends/p4/tableApply.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ limitations under the License.

namespace P4 {

// These
// These are used to figure out whether an expression has the form:
// table.apply().hit
// or
// table.apply().action_run
class TableApplySolver {
public:
static const IR::P4Table* isHit(const IR::Expression* expression,
Expand Down
4 changes: 2 additions & 2 deletions midend/actionSynthesis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ const IR::Node* DoMoveActionsToTables::postorder(IR::MethodCallStatement* statem
// Action invocation
BUG_CHECK(ac->expr->method->is<IR::PathExpression>(),
"%1%: Expected a PathExpression", ac->expr->method);
auto actionPath = new IR::PathExpression(ac->action->name);
auto call = new IR::MethodCallExpression(Util::SourceInfo(), actionPath,
auto actionPath = new IR::PathExpression(IR::ID(mc->srcInfo, ac->action->name));
auto call = new IR::MethodCallExpression(mc->srcInfo, actionPath,
new IR::Vector<IR::Type>(), directionArgs);
auto actinst = new IR::ActionListElement(statement->srcInfo, IR::Annotations::empty, call);
auto actions = new IR::IndexedVector<IR::ActionListElement>();
Expand Down
Loading