Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
  • Loading branch information
asl committed Nov 7, 2024
1 parent fa34687 commit 692b624
Showing 9 changed files with 27 additions and 26 deletions.
2 changes: 1 addition & 1 deletion backends/bmv2/common/backend.h
Original file line number Diff line number Diff line change
@@ -170,7 +170,7 @@ class RenameUserMetadata : public Transform {
cstring newName = namePrefix + suffix;
LOG2("Renaming " << f << " to " << newName);
auto field = new IR::StructField(
f->srcInfo, f->name, IR::Annotations::setNameAnnotation(newName, f->annotations),
f->srcInfo, f->name, IR::Annotations::setNameAnnotation(f->annotations, newName),
f->type);
fields.push_back(field);
}
2 changes: 1 addition & 1 deletion control-plane/flattenHeader.cpp
Original file line number Diff line number Diff line change
@@ -43,7 +43,7 @@ void FlattenHeader::doFlatten(const IR::Type *type) {
auto originalName = makeName(".");
auto annotations = mergeAnnotations();
auto field = new IR::StructField(
IR::ID(newName), IR::Annotations::setNameAnnotation(originalName, annotations), type);
IR::ID(newName), IR::Annotations::setNameAnnotation(annotations, originalName), type);
newFields.push_back(field);
auto ftype = typeMap->getTypeType(type, true);
typeMap->setType(field, ftype);
12 changes: 7 additions & 5 deletions frontends/p4-14/fromv1.0/programStructure.cpp
Original file line number Diff line number Diff line change
@@ -84,7 +84,7 @@ ProgramStructure::ProgramStructure()

IR::Vector<IR::Annotation> ProgramStructure::addGlobalNameAnnotation(
cstring name, const IR::Vector<IR::Annotation> &annos) {
return IR::Annotations::addNameAnnotation(absl::StrCat(".", name), annos);
return IR::Annotations::maybeAddNameAnnotation(annos, absl::StrCat(".", name));
}

cstring ProgramStructure::makeUniqueName(cstring base) {
@@ -175,7 +175,7 @@ cstring ProgramStructure::createType(const IR::Type_StructLike *type, bool heade
auto type_name = types.get(type);
auto newType = type->apply(TypeConverter(this));
if (newType->name.name != type_name) {
auto annos = IR::Annotations::addNameAnnotation(type->name.name, type->annotations);
auto annos = IR::Annotations::maybeAddNameAnnotation(type->annotations, type->name.name);
if (header) {
newType =
new IR::Type_Header(newType->srcInfo, type_name, std::move(annos), newType->fields);
@@ -239,7 +239,8 @@ void ProgramStructure::createTypes() {
// Registers always use struct types
auto newType = new IR::Type_Struct(
type->srcInfo, type_name,
IR::Annotations::addNameAnnotation(layoutTypeName, type->annotations), st->fields);
IR::Annotations::maybeAddNameAnnotation(type->annotations, layoutTypeName),
st->fields);
checkHeaderType(newType, false);
LOG3("Added type " << dbp(newType) << " named " << type_name << " from " << dbp(type));
declarations->push_back(newType);
@@ -277,7 +278,7 @@ const IR::Type_Struct *ProgramStructure::createFieldListType(const IR::Expressio

auto name = makeUniqueName(nr->path->name.name);
auto result = new IR::Type_Struct(expression->srcInfo, name,
IR::Annotations::addNameAnnotation(nr->path->name, {}));
IR::Annotations::maybeAddNameAnnotation({}, nr->path->name));
std::set<cstring> fieldNames;
int field_id = 0;
for (auto f : fl->fields) {
@@ -983,7 +984,8 @@ const IR::P4Table *ProgramStructure::convertTable(const IR::V1Table *table, cstr
name = bin->right->toString();
else if (bin->right->is<IR::Constant>())
name = bin->left->toString();
if (!name.isNullOrEmpty()) annos = IR::Annotations::addNameAnnotation(name, annos);
if (!name.isNullOrEmpty())
annos = IR::Annotations::maybeAddNameAnnotation(annos, name);
}
// Here we rely on the fact that the spelling of 'rt' is
// the same in P4-14 and core.p4/v1model.p4.
12 changes: 7 additions & 5 deletions frontends/p4/inlining.cpp
Original file line number Diff line number Diff line change
@@ -244,7 +244,7 @@ class Substitutions : public SubstituteParameters {
cstring extName = renameMap->getExtName(orig);
LOG3("Renaming " << dbp(orig) << " to " << newName << " (" << extName << ")");
return new IR::P4Table(table->srcInfo, newName,
IR::Annotations::setNameAnnotation(extName, table->annotations),
IR::Annotations::setNameAnnotation(table->annotations, extName),
table->properties);
}
const IR::Node *postorder(IR::P4ValueSet *set) override {
@@ -253,7 +253,7 @@ class Substitutions : public SubstituteParameters {
cstring extName = renameMap->getExtName(orig);
LOG3("Renaming " << dbp(orig) << " to " << newName << "(" << extName << ")");
return new IR::P4ValueSet(set->srcInfo, newName,
IR::Annotations::setNameAnnotation(extName, set->annotations),
IR::Annotations::setNameAnnotation(set->annotations, extName),
set->elementType, set->size);
}
const IR::Node *postorder(IR::P4Action *action) override {
@@ -262,7 +262,7 @@ class Substitutions : public SubstituteParameters {
cstring extName = renameMap->getExtName(orig);
LOG3("Renaming " << dbp(orig) << " to " << newName << "(" << extName << ")");
return new IR::P4Action(action->srcInfo, newName,
IR::Annotations::setNameAnnotation(extName, action->annotations),
IR::Annotations::setNameAnnotation(action->annotations, extName),
action->parameters, action->body);
}
const IR::Node *postorder(IR::Declaration_Instance *instance) override {
@@ -271,7 +271,8 @@ class Substitutions : public SubstituteParameters {
cstring extName = renameMap->getExtName(orig);
LOG3("Renaming " << dbp(orig) << " to " << newName << "(" << extName << ")");
instance->name = newName;
instance->annotations = IR::Annotations::setNameAnnotation(extName, instance->annotations);
IR::Annotations::addOrReplace(instance->annotations, IR::Annotation::nameAnnotation,
new IR::StringLiteral(extName));
return instance;
}
const IR::Node *postorder(IR::Declaration_Variable *decl) override {
@@ -280,7 +281,8 @@ class Substitutions : public SubstituteParameters {
cstring extName = renameMap->getExtName(orig);
LOG3("Renaming " << dbp(orig) << " to " << newName << "(" << extName << ")");
decl->name = newName;
decl->annotations = IR::Annotations::setNameAnnotation(extName, decl->annotations);
IR::Annotations::addOrReplace(decl->annotations, IR::Annotation::nameAnnotation,
new IR::StringLiteral(extName));
return decl;
}
const IR::Node *postorder(IR::PathExpression *expression) override {
4 changes: 2 additions & 2 deletions frontends/p4/localizeActions.cpp
Original file line number Diff line number Diff line change
@@ -78,7 +78,7 @@ bool FindGlobalActionUses::preorder(const IR::PathExpression *path) {

auto replacement = new IR::P4Action(
action->srcInfo, IR::ID(action->name.srcInfo, newName, action->name.originalName),
IR::Annotations::addNameAnnotation(action->name, action->annotations), params,
IR::Annotations::maybeAddNameAnnotation(action->annotations, action->name), params,
replBody);
repl->addReplacement(action, control, replacement);
}
@@ -165,7 +165,7 @@ bool FindRepeatedActionUses::preorder(const IR::PathExpression *expression) {

replacement = new IR::P4Action(
action->srcInfo, IR::ID(action->name.srcInfo, newName, action->name.originalName),
IR::Annotations::addNameAnnotation(action->name, action->annotations), params,
IR::Annotations::maybeAddNameAnnotation(action->annotations, action->name), params,
replBody);
repl->createReplacement(action, actionUser, replacement);
}
6 changes: 2 additions & 4 deletions frontends/p4/sideEffects.h
Original file line number Diff line number Diff line change
@@ -118,10 +118,8 @@ class SideEffects : public Inspector {
// * isValid()
// * function, extern function, or extern method with noSideEffects annotation
auto mi = MethodInstance::resolve(mce, refMap, typeMap);
if (const auto *em = mi->to<P4::ExternMethod>())
return !em->method->hasAnnotation(IR::Annotation::noSideEffectsAnnotation);
if (const auto *ef = mi->to<P4::ExternFunction>())
return !ef->method->hasAnnotation(IR::Annotation::noSideEffectsAnnotation);
if (const auto *ec = mi->to<P4::ExternCall>())
return !ec->method->hasAnnotation(IR::Annotation::noSideEffectsAnnotation);
if (const auto *ef = mi->to<P4::FunctionCall>())
return !ef->function->hasAnnotation(IR::Annotation::noSideEffectsAnnotation);
if (const auto *bim = mi->to<BuiltInMethod>()) return bim->name != IR::Type_Header::isValid;
4 changes: 2 additions & 2 deletions ir/annotations.cpp
Original file line number Diff line number Diff line change
@@ -64,13 +64,13 @@ void addOrReplace(Vector<Annotation> &annotations, const IR::Annotation *ann) {
annotations.push_back(ann);
}

Vector<Annotation> addNameAnnotation(cstring name, const Vector<Annotation> &annos) {
Vector<Annotation> maybeAddNameAnnotation(const Vector<Annotation> &annos, cstring name) {
auto result = annos;
Annotations::addIfNew(result, IR::Annotation::nameAnnotation, new IR::StringLiteral(name));
return result;
}

Vector<Annotation> setNameAnnotation(cstring name, const Vector<Annotation> &annos) {
Vector<Annotation> setNameAnnotation(const Vector<Annotation> &annos, cstring name) {
auto result = annos;
Annotations::addOrReplace(result, IR::Annotation::nameAnnotation, new IR::StringLiteral(name));
return result;
5 changes: 3 additions & 2 deletions ir/annotations.h
Original file line number Diff line number Diff line change
@@ -23,9 +23,10 @@ class Expression;

namespace Annotations {
/// Add @name annotation if there is no annotation already set
[[nodiscard]] Vector<Annotation> addNameAnnotation(cstring name, const Vector<Annotation> &annos);
[[nodiscard]] Vector<Annotation> maybeAddNameAnnotation(const Vector<Annotation> &annos,
cstring name);
/// Replaces @name annotation, if any. Otherwise - add the name annotation
[[nodiscard]] Vector<Annotation> setNameAnnotation(cstring name, const Vector<Annotation> &annos);
[[nodiscard]] Vector<Annotation> setNameAnnotation(const Vector<Annotation> &annos, cstring name);
[[nodiscard]] Vector<Annotation> withoutNameAnnotation(const Vector<Annotation> &annos);

void addIfNew(Vector<Annotation> &annotations, cstring name, const Expression *expr,
6 changes: 2 additions & 4 deletions midend/flattenInterfaceStructs.h
Original file line number Diff line number Diff line change
@@ -72,11 +72,9 @@ struct StructTypeReplacement : public IHasDbPrint {
auto vec = new IR::IndexedVector<IR::StructField>();
flatten(typeMap, cstring::empty, type, type->annotations, vec, policy);
if (type->is<IR::Type_Struct>()) {
replacementType =
new IR::Type_Struct(type->srcInfo, type->name, IR::Vector<IR::Annotation>(), *vec);
replacementType = new IR::Type_Struct(type->srcInfo, type->name, *vec);
} else if (type->is<IR::Type_Header>()) {
replacementType =
new IR::Type_Header(type->srcInfo, type->name, IR::Vector<IR::Annotation>(), *vec);
replacementType = new IR::Type_Header(type->srcInfo, type->name, *vec);
} else {
BUG("Unexpected type %1%", type);
}

0 comments on commit 692b624

Please sign in to comment.