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

Remove unnecessary check in conversion of log_msg to a JSON #3067

Merged
merged 18 commits into from
Mar 4, 2022
Merged
Show file tree
Hide file tree
Changes from 17 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
3 changes: 0 additions & 3 deletions backends/bmv2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ set (XFAIL_TESTS
testdata/p4_16_samples/issue986-bmv2.p4
# These tests use a table key with type 'error'
testdata/p4_16_samples/issue1062-bmv2.p4
testdata/p4_16_samples/issue1062-1-bmv2.p4
# This test uses an undefined extern
testdata/p4_16_samples/issue1193-bmv2.p4
# This test also uses a custom extern
Expand All @@ -182,8 +181,6 @@ set (XFAIL_TESTS
testdata/p4_16_samples/issue1882-bmv2.p4
testdata/p4_16_samples/issue1882-1-bmv2.p4
testdata/p4_16_samples/issue2664-bmv2.p4
# This one uses arguments of incorrect types to log_msg
testdata/p4_16_samples/issue2201-bmv2.p4
)

set (BMV2_PARSER_INLINE_TESTS "${P4C_SOURCE_DIR}/testdata/p4_16_samples/parser-inline/*.p4")
Expand Down
5 changes: 3 additions & 2 deletions backends/bmv2/common/control.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,10 @@ class ControlConverter : public Inspector {
for (auto ke : key->keyElements) {
auto expr = ke->expression;
auto ket = ctxt->typeMap->getType(expr, true);
if (!ket->is<IR::Type_Bits>() && !ket->is<IR::Type_Boolean>())
if (!ket->is<IR::Type_Bits>() && !ket->is<IR::Type_Boolean>() &&
!ket->is<IR::Type_Error>())
::error(ErrorType::ERR_UNSUPPORTED, "%1%: unsupporded key type %2%. "
"Supported key types are be bit<> or boolean.", expr, ket);
"Supported key types are be bit<> or boolean, or error.", expr, ket);

auto match_type = getKeyMatchType(ke);
if (match_type == BMV2::MatchImplementation::selectorMatchTypeName)
Expand Down
5 changes: 4 additions & 1 deletion backends/bmv2/simple_switch/simpleSwitch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ limitations under the License.
#include "backends/bmv2/common/annotations.h"
#include "frontends/p4/fromv1.0/v1model.h"
#include "frontends/p4/cloner.h"
#include "midend/flattenLogMsg.h"
#include "simpleSwitch.h"
#include "backends/bmv2/simple_switch/options.h"

Expand Down Expand Up @@ -858,7 +859,8 @@ Util::IJson* ExternConverter_log_msg::convertExternFunction(
auto arr = new Util::JsonArray();
for (auto v : le->components) {
auto tf = ctxt->typeMap->getType(v);
if (!tf->is<IR::Type_Bits>() && !tf->is<IR::Type_Boolean>()) {
if (!tf->is<IR::Type_Bits>() && !tf->is<IR::Type_Boolean>() &&
!tf->is<IR::Type_Error>()) {
::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,
"%1%: only integral values supported for logged values", mc);
return primitive;
Expand Down Expand Up @@ -1169,6 +1171,7 @@ SimpleSwitchBackend::convert(const IR::ToplevelBlock* tlb) {
new ProcessControls(&structure->pipeline_controls)),
new P4::SimplifyControlFlow(refMap, typeMap),
new P4::RemoveAllUnusedDeclarations(refMap),
new P4::FlattenLogMsg(refMap, typeMap),
// Converts the DAG into a TREE (at least for expressions)
// This is important later for conversion to JSON.
new P4::ClonePathExpressions(),
Expand Down
1 change: 1 addition & 0 deletions midend/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ set (MIDEND_SRCS
fillEnumMap.cpp
flattenHeaders.cpp
flattenInterfaceStructs.cpp
flattenLogMsg.cpp
hsIndexSimplify.cpp
interpreter.cpp
global_copyprop.cpp
Expand Down
199 changes: 199 additions & 0 deletions midend/flattenLogMsg.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
#include "midend/flattenLogMsg.h"
#include "flattenLogMsg.h"

namespace P4 {

bool FindTypesInLogMsgInvocationToReplace::hasStructInParameter(
const IR::MethodCallStatement* methodCallStatement) {
if (auto* path = methodCallStatement->methodCall->method->to<IR::PathExpression>()) {
if (methodCallStatement->methodCall->arguments->size() == 2 &&
path->path->name.name == "log_msg") {
auto* param1 = methodCallStatement->methodCall->arguments->at(0)->expression;
VolodymyrPeschanenkoIntel marked this conversation as resolved.
Show resolved Hide resolved
auto* param2 = methodCallStatement->methodCall->arguments->at(1)->expression;
if (!param1->is<IR::StringLiteral>() || !param2->is<IR::StructExpression>()) {
return false;
}
auto* type =
typeMap->getTypeType(methodCallStatement->methodCall->typeArguments->at(0), true);
if (auto* typeStruct = type->to<IR::Type_StructLike>()) {
for (auto field : typeStruct->fields) {
if (field->type->is<IR::Type_StructLike>()) {
return true;
}
}
}
}
}
return false;
}

const IR::MethodCallStatement* FindTypesInLogMsgInvocationToReplace::prepareLogMsgStatement(
const IR::MethodCallStatement* methodCallStatement) {
if (auto* newMethod = getReplacementMethodCall(methodCallStatement->id)) {
return newMethod;
}
// Create new statement
auto* param1 = methodCallStatement->methodCall->arguments->at(0)->expression;
std::string strParam1 = param1->to<IR::StringLiteral>()->value.c_str();
auto* argType =
mihaibudiu marked this conversation as resolved.
Show resolved Hide resolved
typeMap->getTypeType(methodCallStatement->methodCall->typeArguments->at(0), true);
auto* structType = argType->checkedTo<IR::Type_StructLike>();
index = 0;
auto exprVector =
unfoldStruct(methodCallStatement->methodCall->arguments->at(1)->expression, strParam1);
structType = generateNewStructType(structType, exprVector.first);
auto* newMethodCall = methodCallStatement->methodCall->clone();
mihaibudiu marked this conversation as resolved.
Show resolved Hide resolved
auto* newTypeArguments = newMethodCall->typeArguments->clone();
newTypeArguments->clear();
newTypeArguments->push_back(structType);
newMethodCall->typeArguments = newTypeArguments;
auto* newArguments = newMethodCall->arguments->clone();
auto* newArgument = newMethodCall->arguments->at(0)->clone();
auto* newString = new IR::StringLiteral(newArgument->expression->srcInfo, exprVector.second);
newArgument->expression = newString;
newArguments->at(0) = newArgument;
newArgument = newMethodCall->arguments->at(1)->clone();
auto* oldType = newArgument->expression->to<IR::StructExpression>()->structType;
auto newStructExpression =new IR::StructExpression(oldType->srcInfo, structType,
structType->getP4Type()->template to<IR::Type_Name>(), exprVector.first);
newStructExpression->components = exprVector.first;
newArgument->expression = newStructExpression;
newArguments->at(1) = newArgument;
newMethodCall->arguments = newArguments;
auto* newMethodCallStatement = methodCallStatement->clone();
newMethodCallStatement->methodCall = newMethodCall;
logMsgReplacement.emplace(getOriginal()->id, newMethodCallStatement);
return newMethodCallStatement;
}


IR::ID FindTypesInLogMsgInvocationToReplace::newName() {
std::ostringstream ostr;
ostr << "f" << index++;
return IR::ID(ostr.str());
}

const IR::Type_StructLike* FindTypesInLogMsgInvocationToReplace::generateNewStructType(
const IR::Type_StructLike* structType, IR::IndexedVector<IR::NamedExpression> &v) {
IR::IndexedVector<IR::StructField> fields;
for (auto namedExpr : v) {
auto* structField = new IR::StructField(namedExpr->srcInfo, namedExpr->name,
namedExpr->expression->type);
typeMap->setType(structField, structField->type);
VolodymyrPeschanenkoIntel marked this conversation as resolved.
Show resolved Hide resolved
fields.push_back(structField);
}
auto* newType = structType->clone();
newType->fields = fields;
return newType;
}

TypeLogMsgParams FindTypesInLogMsgInvocationToReplace::unfoldStruct(const IR::Expression* expr,
std::string strParam, std::string curName) {
TypeLogMsgParams result;
auto* exprType = typeMap->getType(expr, true);
if (!expr->is<IR::StructExpression>()) {
if (auto structType = exprType->to<IR::Type_StructLike>()) {
result.second += "(";
for (auto field : structType->fields) {
std::string nm = field->name.name + std::string(":");
auto* newMember = new IR::Member(field->type, expr, field->name);
if (field->type->is<IR::Type_StructLike>()) {
auto curResult = unfoldStruct(newMember, strParam, field->name.name.c_str());
nm += curResult.second;
result.first.insert(result.first.end(), curResult.first.begin(),
curResult.first.end());
} else {
result.first.push_back(new IR::NamedExpression(expr->srcInfo, newName(),
newMember));
nm += std::string("{}");
}
if (result.second.length() > 1)
result.second += ",";
result.second += nm;
}
result.second += std::string(")");
return result;
}
result.first.push_back(new IR::NamedExpression(expr->srcInfo, newName(), expr));
result.second = curName + std::string(" : {}");
return result;
}
auto* structExpr = expr->to<IR::StructExpression>();
std::string strResult;
for (auto namedExpr : structExpr->components) {
size_t n = strParam.find("{}");
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if you don't find this substring?
should you give a warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should add other arguments in StructExpression to this {} like in your example below.

if (n != std::string::npos) {
strResult += strParam.substr(0, n);
strParam = strParam.substr(n + 2);
} else {
BUG("Can't find '{}' in '%1%'", expr);
}
exprType = typeMap->getType(namedExpr->expression, true);
if (exprType->is<IR::Type_StructLike>()) {
std::string nm;
if (curName.length()) {
nm = curName + std::string(".");
}
nm += namedExpr->name.name.c_str();
auto innerFields = unfoldStruct(namedExpr->expression, "", nm);
result.first.insert(result.first.end(), innerFields.first.begin(),
innerFields.first.end());
strResult += innerFields.second;
} else {
strResult += "{}";
result.first.push_back(
new IR::NamedExpression(namedExpr->srcInfo, newName(), namedExpr->expression));
}
}
result.second = strResult + strParam;
return result;
}

bool FindTypesInLogMsgInvocationToReplace::preorder(
const IR::MethodCallStatement* methodCallStatement) {
if (hasStructInParameter(methodCallStatement)) {
auto* newMethodCall = prepareLogMsgStatement(methodCallStatement);
createReplacement(newMethodCall->methodCall->typeArguments->at(0)->
to<IR::Type_Struct>());
}
return false;
}

void FindTypesInLogMsgInvocationToReplace::createReplacement(const IR::Type_StructLike* type) {
if (replacement.count(type->name)) {
return;
}
replacement.emplace(type->name, type);
}

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

const IR::Node* ReplaceLogMsg::preorder(IR::P4Program* program) {
if (findTypesInLogMsgInvocationToReplace->empty()) {
// nothing to do
prune();
}
return program;
}

const IR::Node* ReplaceLogMsg::postorder(IR::Type_Struct* typeStruct) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you replace the type? If this type is used in other places besides the logmsg call it should remain in the program. So you should append the new type, not replace the old one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you, please, suggest a place where is I can to insert new type?

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 think that this type couldn't be used in other places, because it should be unique for each log_msg function and each log_msg function will be replace for new one.

Copy link
Contributor

Choose a reason for hiding this comment

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

The type you generate is new, but you could have something like this:

struct S { bit<8> f; } 
...
S s;
log_msg("{}", {s,s});

In this case you don't want to remove the type S and replace it with your new generated type.
The new type should be inserted right before the control/parser/action that calls log_msg.

auto canon = typeMap->getTypeType(getOriginal(), true);
auto name = canon->to<IR::Type_Struct>()->name;
auto repl = findTypesInLogMsgInvocationToReplace->getReplacement(name);
if (repl != nullptr) {
LOG3("Replace " << typeStruct << " with " << repl);
BUG_CHECK(repl->is<IR::Type_Struct>(), "%1% not a struct", typeStruct);
return repl;
}
return typeStruct;
}

const IR::Node* ReplaceLogMsg::postorder(IR::MethodCallStatement* methodCallStatement) {
if (auto* newMethod = findTypesInLogMsgInvocationToReplace->
getReplacementMethodCall(getOriginal()->id)) {
return newMethod;
}
return methodCallStatement;
}

} // namespace P4
94 changes: 94 additions & 0 deletions midend/flattenLogMsg.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
#ifndef _MIDEND_FLATTENILOGMSG_H_
#define _MIDEND_FLATTENILOGMSG_H_

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

namespace P4 {

using TypeLogMsgParams = std::pair<IR::IndexedVector<IR::NamedExpression>, std::string>;

/*
Find types in log_msg function for replace.
*/
class FindTypesInLogMsgInvocationToReplace : public Inspector {
P4::TypeMap* typeMap;
ordered_map<cstring, const IR::Type_StructLike*> replacement;
// Map with new log_msg methods where is each key is a original identifier of IR::Node.
ordered_map<unsigned, const IR::MethodCallStatement* > logMsgReplacement;
size_t index;

public:
explicit FindTypesInLogMsgInvocationToReplace(P4::TypeMap *typeMap) : typeMap(typeMap) {
setName("FindTypesInLogMsgInvocationToReplace");
CHECK_NULL(typeMap);
}
bool preorder(const IR::MethodCallStatement* methodCallStatement) override;
void createReplacement(const IR::Type_StructLike* type);
const IR::MethodCallStatement* prepareLogMsgStatement(
const IR::MethodCallStatement* methodCallStatement);
const IR::Type_StructLike* getReplacement(const cstring name) const {
return ::get(replacement, name); }
const IR::MethodCallStatement* getReplacementMethodCall(unsigned id) const {
return ::get(logMsgReplacement, id);
}
bool empty() const { return replacement.empty(); }
bool hasStructInParameter(const IR::MethodCallStatement* methodCallStatement);

protected:
TypeLogMsgParams unfoldStruct(const IR::Expression* expr, std::string strParam,
std::string curName = "");
const IR::Type_StructLike* generateNewStructType(const IR::Type_StructLike* structType,
IR::IndexedVector<IR::NamedExpression> &v);
IR::ID newName();
};

/**
This pass translates all occuarence of log_msg function with non-flattend arguments into
set of the flatten calls of log_smg.
For example,
struct alt_t {
bit<1> valid;
bit<7> port;
}
...
t : slt_t;
...
log_msg("t={}", {t});

The flattened log_msg is shown below.

log_msg("t=(valid:{},port:{})", {t.valid, t.port});
*/
class ReplaceLogMsg : public Transform, P4WriteContext {
P4::TypeMap* typeMap;
FindTypesInLogMsgInvocationToReplace* findTypesInLogMsgInvocationToReplace;
public:
explicit ReplaceLogMsg(P4::TypeMap* typeMap,
FindTypesInLogMsgInvocationToReplace* findTypesInLogMsgInvocationToReplace)
: typeMap(typeMap),
findTypesInLogMsgInvocationToReplace(findTypesInLogMsgInvocationToReplace) {
CHECK_NULL(typeMap); CHECK_NULL(findTypesInLogMsgInvocationToReplace);
setName("ReplaceLogMsg");
}
const IR::Node* preorder(IR::P4Program* program);
const IR::Node* postorder(IR::MethodCallStatement* methodCallStatement) override;
const IR::Node* postorder(IR::Type_Struct* typeStruct) override;
};

class FlattenLogMsg final : public PassManager {
public:
FlattenLogMsg(P4::ReferenceMap* refMap, P4::TypeMap* typeMap) {
auto findTypesInLogMsgInvocationToReplace =
new FindTypesInLogMsgInvocationToReplace(typeMap);
passes.push_back(new TypeChecking(refMap, typeMap));
passes.push_back(findTypesInLogMsgInvocationToReplace);
passes.push_back(new ReplaceLogMsg(typeMap, findTypesInLogMsgInvocationToReplace));
passes.push_back(new ClearTypeMap(typeMap));
setName("FlattenLogMsg");
}
};

} // namespace P4

#endif /* _MIDEND_FLATTENILOGMSG_H_ */
13 changes: 13 additions & 0 deletions testdata/p4_16_samples/issue1062-1-bmv2_0.stf
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# STF test for /home/vpeschax/networking.switching.barefoot.bf-p4c-tools/res/issue1062-1-bmv2
# p4testgen seed: 1000

# Traces
# p: [Parser] p
# [State] start
# [Event]: Extract Reject
# [Event]: Extract Cond: p4t*zombie.const.0.*packetLen_bits >= 48;
# [Expression]: Invalid emit: h.h.*valid;: = 0;
# [Event]: Prepending the emit buffer to the program packet.

packet 0 00000000
expect 0 00000000$