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

Doxygen comments #614

Merged
merged 15 commits into from
May 11, 2017
21 changes: 1 addition & 20 deletions midend/moveConstructors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,26 +29,7 @@ struct ConstructorMap {
};

namespace {
// Converts constructor call expressions that appear
// within the bodies of P4Parser and P4Control blocks
// into Declaration_Instance. This is needed for implementing
// copy-in/copy-out in inlining, since
// constructed objects do not have assignment operations.
// For example:
// extern T {}
// control c()(T t) { apply { ... } }
// control d() {
// c(T()) cinst;
// apply { ... }
// }
// is converted to
// extern T {}
// control c()(T t) { apply { ... } }
// control d() {
// T() tmp;
// c(tmp) cinst;
// apply { ... }
// }

class MoveConstructorsImpl : public Transform {
enum class Region {
InParserStateful,
Expand Down
28 changes: 28 additions & 0 deletions midend/moveConstructors.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,34 @@ limitations under the License.

namespace P4 {

/**
* This pass converts constructor call expressions that appear
* within the bodies of P4Parser and P4Control blocks
* into Declaration_Instance. This is needed for implementing
* copy-in/copy-out in inlining, since
* constructed objects do not have assignment operations.
*
* For example:
* extern T {}
* control c()(T t) { apply { ... } }
* control d() {
* c(T()) cinst;
* apply { ... }
* }
* is converted to
* extern T {}
* control c()(T t) { apply { ... } }
* control d() {
* T() tmp;
* c(tmp) cinst;
* apply { ... }
* }
*
* @pre None
* @post No constructor call expression in P4Parser and P4Control
* constructor parameters.
*
*/
class MoveConstructors : public PassManager {
public:
explicit MoveConstructors(ReferenceMap* refMap);
Expand Down
11 changes: 10 additions & 1 deletion midend/removeLeftSlices.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,16 @@ limitations under the License.

namespace P4 {

// Remove Slices on the lhs of an assignment
/**
* This pass removes Slices on the lhs of an assignment
*
* \code{.cpp}
* a[m:l] = e; -> a = (a & ~mask) | (((cast)e << l) & mask);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! The example illustrates this transformation perfectly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Will this be typeset as code in Doxygen?

* \endcode
*
* @pre none
* @post no field slice operator in the lhs of assignment statement
*/
class DoRemoveLeftSlices : public Transform {
P4::TypeMap* typeMap;
public:
Expand Down
6 changes: 2 additions & 4 deletions midend/removeParameters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,19 +148,17 @@ const IR::Node* DoRemoveActionParameters::postorder(IR::MethodCallExpression* ex
return expression;
}

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

RemoveActionParameters::RemoveActionParameters(ReferenceMap* refMap, TypeMap* typeMap) {
setName("RemoveActionParameters");
// This is needed because of this case:
auto ai = new ActionInvocation();
// MoveDeclarations() is needed because of this case:
// action a(inout x) { x = x + 1 }
// bit<32> w;
// table t() { actions = a(w); ... }
// Without the MoveDeclarations the code would become
// action a() { x = w; x = x + 1; w = x; } << w is not yet defined
// bit<32> w;
// table t() { actions = a(); ... }
auto ai = new ActionInvocation();
passes.emplace_back(new MoveDeclarations());
passes.emplace_back(new TypeChecking(refMap, typeMap));
passes.emplace_back(new FindActionParameters(refMap, typeMap, ai));
Expand Down
46 changes: 30 additions & 16 deletions midend/removeParameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@ limitations under the License.

namespace P4 {

// For each action that is invoked keep the list of arguments that it's called with.
// There must be only one call of each action; this is done by LocalizeActions.
/**
* For each action that is invoked keep the list of arguments that
* it's called with. There must be only one call of each action;
* this is done by LocalizeActions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to use @pre and @post

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

*/
class ActionInvocation {
std::map<const IR::P4Action*, const IR::MethodCallExpression*> invocations;
std::set<const IR::P4Action*> all; // for these actions remove all parameters
std::set<const IR::MethodCallExpression*> calls;
// how many arguments to remove from each default action
/// how many arguments to remove from each default action
std::map<const IR::MethodCallExpression*, unsigned> defaultActions;

public:
Expand Down Expand Up @@ -83,19 +86,30 @@ class FindActionParameters : public Inspector {
void postorder(const IR::MethodCallExpression* expression) override;
};

// Removes parameters of an action which are in/inout/out.
// For this to work each action must have a single caller.
// (This is done by the LocalizeActions pass).
// control c(inout bit<32> x) {
// action a(in bit<32> arg) { x = arg; }
// table t() { actions = { a(10); } }
// apply { ... } }
// is converted to
// control c(inout bit<32> x) {
// bit<32> arg;
// action a() { arg = 10; x = arg; }
// table t() { actions = { a; } }
// apply { ... } }
/**
* Removes parameters of an action which are in/inout/out.
*
* \code{.cpp}
* control c(inout bit<32> x) {
* action a(in bit<32> arg) { x = arg; }
* table t() { actions = { a(10); } }
* apply { ... } }
* \endcode
*
* is converted to
*
* \code{.cpp}
* control c(inout bit<32> x) {
* bit<32> arg;
* action a() { arg = 10; x = arg; }
* table t() { actions = { a; } }
* apply { ... } }
* \endcode
*
* @pre This pass requires each action to have a single caller.
* It must run after the LocalizeActions pass.
* @post in/inout/out parameters of an action are removed.
*/
class DoRemoveActionParameters : public Transform {
ActionInvocation* invocations;
public:
Expand Down
53 changes: 46 additions & 7 deletions midend/simplifyKey.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,19 @@ limitations under the License.

namespace P4 {

// Policy used to decide whether a key expression is too complex.
/**
* Policy used to decide whether a key expression is too complex.
*/
class KeyIsComplex {
public:
virtual ~KeyIsComplex() {}
virtual bool isTooComplex(const IR::Expression* expression) const = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

The isTooComplex function requires a comment.

};

// Policy which returns 'true' whenever a key is not a left value
// or a call to the isValid().
/**
* Policy that tracks whether a key is not a left value or a call to the isValid().
* It returns 'true' if any of the conditions is met.
*/
class NonLeftValue : public KeyIsComplex {
ReferenceMap* refMap;
TypeMap* typeMap;
Expand All @@ -42,7 +46,9 @@ class NonLeftValue : public KeyIsComplex {
bool isTooComplex(const IR::Expression* expression) const;
Copy link
Contributor

Choose a reason for hiding this comment

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

How does "too complex" related to l-value-ness?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pass applies to non-lvalue expressions. All lvalues are considered not complex and therefore not transformed.

};

// Policy that allows masked lvalues as well as simple lvalues or isValid()
/**
* Policy that tracks whether a expression is masked non-lvalue, simple non-lvalue or isValid()
*/
class NonMaskLeftValue : public NonLeftValue {
public:
NonMaskLeftValue(ReferenceMap* refMap, TypeMap* typeMap) : NonLeftValue(refMap, typeMap) {}
Expand All @@ -61,8 +67,36 @@ class TableInsertions {
std::vector<const IR::AssignmentStatement*> statements;
};

// If some of the fields of a table key are "too complex",
// turn them into simpler expressions.
/**
* Transform complex table keys into simpler expressions.
*
* \code{.cpp}
* table t {
* key = { h.a + 1 : exact; }
* ...
* }
* apply {
* t.apply();
* }
* \endcode
*
* is transformed to
*
* \code{.cpp}
* bit<32> key_0;
* table t {
* key = { key_0 : exact; }
* ...
* }
* apply {
* key_0 = h.a + 1;
* t.apply();
* }
* \endcode
*
* @pre none
* @post all complex non-lvalue table key expressions are replaced with a lvalue expression.
*/
class DoSimplifyKey : public Transform {
ReferenceMap* refMap;
TypeMap* typeMap;
Expand All @@ -86,7 +120,12 @@ class DoSimplifyKey : public Transform {
const IR::Node* postorder(IR::P4Table* table) override;
};

// Use a policy to decide when a key expression is too complex.
/**
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 incorrect, this pass actually simplifies the keys, it does not determine anything.
You deleted the original comment, which indicated what the policy parameter is about; that should become a @param comment.

* This pass uses 'policy' to determine whether a key expression is too complex.
* Policies are defined in 'NonLeftValue' and 'NonMaskLeftValue', which consider
Copy link
Contributor

Choose a reason for hiding this comment

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

This pass should not list all possible policies; we can't be updating this comment every time we write a new policy.
Each policy has its own comments. This pass also simplifies keys which are complex according to the policy.

* any non-lvalue, except built-in method (isValid) in table key expressions
* as complex expressions.
*/
class SimplifyKey : public PassManager {
public:
SimplifyKey(ReferenceMap* refMap, TypeMap* typeMap, const KeyIsComplex* policy) {
Expand Down
17 changes: 13 additions & 4 deletions midend/simplifySelectCases.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,19 @@ limitations under the License.

namespace P4 {

// Analyze reachability of select cases; handles corner cases
// (e.g., no cases).
// If requireConstants is true this pass requires that
// all select labels evaluate to constants.
/**
* This pass analyzes reachability of select cases.
* - If there is just one case label, the select statement is eliminated.
* - If a case label appears after the default label, the case is
* unreachable and therefore eliminated.
*
* If requireConstants is true this pass requires that
* all select labels evaluate to constants.
*
* @pre Assumes that there are no side-effects in the evaluation of select labels.
* @post Unreachable case labels are removed. Case statement with
* a single label is replaced with a direct transition
*/
class DoSimplifySelectCases : public Transform {
const TypeMap* typeMap;
bool requireConstants;
Expand Down
9 changes: 7 additions & 2 deletions midend/validateProperties.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ limitations under the License.

namespace P4 {

// Checks to see if there are any unknown properties.
/**
* Checks to see if there are any unknown properties.
*
* @pre none
* @post raise an error if there are invalid table properties in P4 program.
*/
class ValidateTableProperties : public Inspector {
std::set<cstring> legalProperties;
public:
Expand All @@ -36,7 +41,7 @@ class ValidateTableProperties : public Inspector {
legalProperties.emplace(l);
}
void postorder(const IR::Property* property) override;
/* don't check properties in externs (Declaration_Instances) */
// don't check properties in externs (Declaration_Instances)
bool preorder(const IR::Declaration_Instance *) override { return false; }
};

Expand Down