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.

Sign up for GitHub

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 #583 #587

Closed
wants to merge 4 commits into from
Closed

Fix for issue #583 #587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f836c77
Fix for issue #583
May 9, 2017
163e9ae
cpplint fixes
May 9, 2017
f9972cf
Special handling for register layout types
May 10, 2017
36d4da8
Fix for issue #583
May 10, 2017
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
36 changes: 24 additions & 12 deletions frontends/p4-14/typecheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,19 @@ limitations under the License.

#include "typecheck.h"

// P4 v1.0 and v1.1 type checking algorithm
// Initial type setting based on immediate context:
// - replace named reference to ActionParams in the bodies of ActionFunctions with the
// actual ActionParam
// - link header/metadata instances to the Type
// - replace named references to global header or metadata instances with ConcreteHeaderRef
// expressions that link directly to them.
// - set type for Member and HeaderStackItemRefs
/// P4-14 (v1.0 and v1.1) type checking algorithm
/// Initial type setting based on immediate context:
/// - replace named reference to ActionParams in the bodies of ActionFunctions with the
/// actual ActionParam
/// - link header/metadata instances to the Type
/// - replace named references to global header or metadata instances with ConcreteHeaderRef
/// expressions that link directly to them.
/// - set type for Member and HeaderStackItemRefs
class TypeCheck::AssignInitialTypes : public Transform {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any particular reason you're using nested classes rather than namespaces?

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 have only changed the comments here. This code was written by @ChrisDodd.

Copy link
Contributor

Choose a reason for hiding this comment

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

I generally define nested visitors for a PassManager subclass as nested classes in the PassManager subclass; seems cleaner to me that defining a separate namespace in addition to the class.

public:
AssignInitialTypes() { setName("AssignInitialTypes"); }

private:
const IR::V1Program *global = nullptr;

template <typename NodeType, typename TypeType>
Expand Down Expand Up @@ -194,8 +198,12 @@ combineTypes(const Util::SourceInfo &loc, const IR::Type *a, const IR::Type *b)
return a;
}

// bottom up type inferencing -- set the types of expression nodes based on operands
/// Bottom up type inferencing -- set the types of expression nodes based on operands.
class TypeCheck::InferExpressionsBottomUp : public Modifier {
public:
InferExpressionsBottomUp() { setName("InferExpressionsBottomUp"); }

private:
void setType(IR::Expression* currentNode, const IR::Type* type) {
BUG_CHECK(currentNode == getCurrentNode<IR::Expression>(),
"Expected to be called on the visitor's current node");
Expand Down Expand Up @@ -273,8 +281,11 @@ inferTypeFromContext(const Visitor::Context* ctxt, const IR::V1Program* global)
return rv;
}

// top down type inferencing -- set the type of expression nodes based on their uses.
/// Top down type inferencing -- set the type of expression nodes based on their uses.
class TypeCheck::InferExpressionsTopDown : public Modifier {
public:
InferExpressionsTopDown() { setName("InferExpressionsTopDown"); }
private:
const IR::V1Program *global = nullptr;
profile_t init_apply(const IR::Node *root) override {
global = root->to<IR::V1Program>();
Expand Down Expand Up @@ -319,7 +330,7 @@ class TypeCheck::InferActionArgsBottomUp : public Inspector {
prim->typecheck(); } }

public:
explicit InferActionArgsBottomUp(TypeCheck &s) : self(s) {}
explicit InferActionArgsBottomUp(TypeCheck &s) : self(s) { setName("InferActionArgsBottomUp"); }
};

class TypeCheck::InferActionArgsTopDown : public Inspector {
Expand Down Expand Up @@ -360,6 +371,7 @@ class TypeCheck::InferActionArgsTopDown : public Inspector {
// action argument, we need to visit the action argument nodes every
// time they appear in the IR tree.
visitDagOnce = false;
setName("InferActionArgsTopDown");
}
};

Expand Down Expand Up @@ -396,7 +408,7 @@ class TypeCheck::AssignActionArgTypes : public Modifier {
}

public:
explicit AssignActionArgTypes(TypeCheck &s) : self(s) { }
explicit AssignActionArgTypes(TypeCheck &s) : self(s) { setName("AssignActionArgTypes"); }
};

TypeCheck::TypeCheck() : PassManager({
Expand Down
57 changes: 45 additions & 12 deletions frontends/p4/fromv1.0/programStructure.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ void ProgramStructure::checkHeaderType(const IR::Type_StructLike* hdr, bool meta
}
}

void ProgramStructure::createType(const IR::Type_StructLike* type, bool header,
std::unordered_set<const IR::Type*> *converted) {
cstring ProgramStructure::createType(const IR::Type_StructLike* type, bool header,
std::unordered_set<const IR::Type*> *converted) {
if (converted->count(type))
return;
return type->name;
converted->emplace(type);
auto type_name = types.get(type);
auto newType = type->apply(TypeConverter(this));
Expand All @@ -115,6 +115,7 @@ void ProgramStructure::createType(const IR::Type_StructLike* type, bool header,
LOG3("Added type " << dbp(newType) << " named " << type_name << " from " << dbp(type));
declarations->push_back(newType);
converted->emplace(newType);
return type_name;
}

void ProgramStructure::createTypes() {
Expand All @@ -126,14 +127,34 @@ void ProgramStructure::createTypes() {
continue;
createType(type, false, &converted);
}

for (auto it : registers) {
if (it.first->layout) {
auto type = types.get(it.first->layout);
createType(type, false, &converted);
cstring layoutTypeName = it.first->layout;
auto type = types.get(layoutTypeName);
if (converted.count(type) || !type->is<IR::Type_StructLike>())
continue;
auto st = type->to<IR::Type_StructLike>();
if (type->is<IR::Type_Struct>()) {
cstring newName = createType(type, false, &converted);
registerLayoutType.emplace(layoutTypeName, newName);
continue;
}

BUG_CHECK(type->is<IR::Type_Header>(), "%1%: unexpected type", type);
// Must convert to a struct type
cstring type_name = makeUniqueName(st->name);
// Registers always use struct types
auto annos = addNameAnnotation(layoutTypeName, type->annotations);
auto newType = new IR::Type_Struct(type->srcInfo, type_name, annos, st->fields);
checkHeaderType(newType, false);
LOG3("Added type " << dbp(newType) << " named " << type_name << " from " << dbp(type));
declarations->push_back(newType);
registerLayoutType.emplace(layoutTypeName, newType->name);
}
}

// Headers next
// Now headers
converted.clear();
for (auto it : headers) {
auto type = it.first->type;
Expand Down Expand Up @@ -264,6 +285,12 @@ class ProgramStructure::FixupExtern : public Modifier {
};

void ProgramStructure::createExterns() {
for (auto r : registers) {
auto reg = r.first;
if (reg->direct)
::error("%1%: direct registers are not supported yet", reg);
}

for (auto it : extern_types)
declarations->push_back(it.first->apply(FixupExtern(*this, it.second)));
}
Expand Down Expand Up @@ -1276,10 +1303,12 @@ const IR::Statement* ProgramStructure::convertPrimitive(const IR::Primitive* pri
if (!reg) {
::error("Expected a register reference %1%", ref);
return nullptr; }

const IR::Type* castType = nullptr;
int width = reg->width;
if (width <= 0)
width = defaultRegisterWidth;
auto regElementType = IR::Type_Bits::get(width);
if (width > 0)
castType = IR::Type_Bits::get(width);
// Else this is a structured type, so no cast is inserted below.

auto newname = registers.get(reg);
auto registerref = new IR::PathExpression(newname);
Expand All @@ -1288,8 +1317,9 @@ const IR::Statement* ProgramStructure::convertPrimitive(const IR::Primitive* pri
auto args = new IR::Vector<IR::Expression>();
auto arg0 = new IR::Cast(primitive->operands.at(1)->srcInfo, v1model.registers.index_type,
conv.convert(primitive->operands.at(1)));
auto arg1 = new IR::Cast(primitive->operands.at(2)->srcInfo, regElementType,
conv.convert(primitive->operands.at(2)));
const IR::Expression* arg1 = conv.convert(primitive->operands.at(2));
if (castType != nullptr)
arg1 = new IR::Cast(primitive->operands.at(2)->srcInfo, castType, arg1);
args->push_back(arg0);
args->push_back(arg1);
auto mc = new IR::MethodCallExpression(primitive->srcInfo, method, args);
Expand Down Expand Up @@ -1463,7 +1493,10 @@ ProgramStructure::convert(const IR::Register* reg, cstring newName) {
if (reg->width > 0) {
regElementType = IR::Type_Bits::get(reg->width);
} else if (reg->layout) {
regElementType = types.get(reg->layout);
cstring newName = ::get(registerLayoutType, reg->layout);
if (newName.isNullOrEmpty())
newName = reg->layout;
regElementType = new IR::Type_Name(new IR::Path(newName));
} else {
::warning("%1%: Register width unspecified; using %2%", reg, defaultRegisterWidth);
regElementType = IR::Type_Bits::get(defaultRegisterWidth);
Expand Down
7 changes: 5 additions & 2 deletions frontends/p4/fromv1.0/programStructure.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ class ProgramStructure {
/// Type_Header. We can't use the P4-14 type object itself as a
/// key, because it keeps changing.
std::map<cstring, const IR::Type*> finalHeaderType;
/// For registers whose layout is a header, this map contains the mapping
/// from the original layout type name to the final layout type name.
std::map<cstring, cstring> registerLayoutType;

/// Maps each inserted extract statement to the type of the header
/// type that is being extracted. The extracts will need another
Expand Down Expand Up @@ -194,8 +197,8 @@ class ProgramStructure {
void createChecksumUpdates();
void createStructures();
void createExterns();
void createType(const IR::Type_StructLike* type, bool header,
std::unordered_set<const IR::Type*> *converted);
cstring createType(const IR::Type_StructLike* type, bool header,
std::unordered_set<const IR::Type*> *converted);
void createTypes();
void createParser();
void createControls();
Expand Down
2 changes: 1 addition & 1 deletion ir/expression.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class Constant : Literal {
/// if noWarning is true, no warning is emitted
void handleOverflow(bool noWarning);
Constant(intmax_t v, unsigned base = 10) :
Literal(new Type_InfInt()), value(v), base(base) {}
Literal(new Type_InfInt()), value((long)v), base(base) {}
Constant(mpz_class v, unsigned base = 10) :
Literal(new Type_InfInt()), value(v), base(base) {}
Constant(Util::SourceInfo si, mpz_class v, unsigned base = 10) :
Expand Down
Loading