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

Inline some IR methods and constructors #5030

Merged
merged 1 commit into from
Nov 27, 2024
Merged

Inline some IR methods and constructors #5030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions ir/base.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,12 @@ class Path {
ID name;
optional bool absolute = false;
Path { if (!srcInfo) srcInfo = name.srcInfo; }
bool isDontCare() const { return name.isDontCare(); }
inline bool isDontCare() const { return name.isDontCare(); }
toString{
// This is the ORIGINAL name the user used
return absl::StrCat(absolute ? "." : "", name.toString());
}
cstring asString() const {
inline cstring asString() const {
// The CURRENT internal name
return absl::StrCat(absolute ? "." : "", name);
}
Expand Down
118 changes: 59 additions & 59 deletions ir/expression.def
View file
Open in desktop

Large diffs are not rendered by default.

92 changes: 43 additions & 49 deletions ir/ir.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class ParserState : ISimpleNamespace, Declaration, IAnnotated {
static const cstring reject;
static const cstring start;
static const cstring verify;
bool isBuiltin() const { return name == ParserState::accept || name == ParserState::reject; }
inline bool isBuiltin() const { return name == ParserState::accept || name == ParserState::reject; }
validate{
if (selectExpression != nullptr)
BUG_CHECK(selectExpression->is<IR::PathExpression>() ||
Expand Down Expand Up @@ -230,9 +230,9 @@ class ActionListElement : IAnnotated, IDeclaration {
class ActionList : PropertyValue {
inline IndexedVector<ActionListElement> actionList;
validate{ actionList.check_null(); }
size_t size() const { return actionList.size(); }
void push_back(ActionListElement e) { actionList.push_back(e); }
ActionListElement getDeclaration(cstring n) const {
inline size_t size() const { return actionList.size(); }
inline void push_back(ActionListElement e) { actionList.push_back(e); }
inline ActionListElement getDeclaration(cstring n) const {
return actionList.getDeclaration<ActionListElement>(n); }
}

Expand Down Expand Up @@ -267,7 +267,7 @@ class KeyElement : IAnnotated {
class Key : PropertyValue {
inline Vector<KeyElement> keyElements;
validate { keyElements.check_null(); }
void push_back(KeyElement ke) { keyElements.push_back(ke); }
inline void push_back(KeyElement ke) { keyElements.push_back(ke); }
}

/// Pre-defined entry in a table
Expand All @@ -283,15 +283,15 @@ class Entry : IAnnotated {

const Vector<Annotation> &getAnnotations() const override { return annotations; }
Vector<Annotation> &getAnnotations() override { return annotations; }
ListExpression getKeys() const { return keys; }
Expression getAction() const { return action; }
inline ListExpression getKeys() const { return keys; }
inline Expression getAction() const { return action; }
dbprint { out << annotations << keys << action; }
}

/// List of predefined entries. Part of table properties
class EntriesList : PropertyValue {
inline Vector<Entry> entries;
size_t size() const { return entries.size(); }
inline size_t size() const { return entries.size(); }
dbprint { out << "{ " << entries << "}"; }
}

Expand All @@ -309,15 +309,15 @@ class TableProperties : ISimpleNamespace {
toString{ return absl::StrCat("TableProperties(", properties.size(), ")"); }
Util::Enumerator<IDeclaration>* getDeclarations() const override {
return properties.getDeclarations(); }
Property getProperty(cstring name) const {
inline Property getProperty(cstring name) const {
return properties.getDeclaration<Property>(name); }
IDeclaration getDeclByName(cstring name) const override {
return properties.getDeclaration(name); }
Property getProperty(std::string_view name) const {
inline Property getProperty(std::string_view name) const {
return properties.getDeclaration<Property>(name); }
IDeclaration getDeclByName(std::string_view name) const override {
return properties.getDeclaration(name); }
void push_back(Property prop) { properties.push_back(prop); }
inline void push_back(Property prop) { properties.push_back(prop); }

static const cstring actionsPropertyName;
static const cstring keyPropertyName;
Expand All @@ -336,56 +336,50 @@ class P4Table : Declaration, IAnnotated, IApply {
Vector<Annotation> &getAnnotations() override { return annotations; }
Type_Method getApplyMethodType() const override;
ParameterList getApplyParameters() const override { return new ParameterList(); }
ActionList getActionList() const {
auto ap = properties->getProperty(TableProperties::actionsPropertyName);
if (ap == nullptr)
return nullptr;
if (!ap->value->is<IR::ActionList>()) {
inline ActionList getActionList() const {
if (auto ap = properties->getProperty(TableProperties::actionsPropertyName)) {
if (auto al = ap->value->to<IR::ActionList>()) return al;
::P4::error(ErrorType::ERR_INVALID, "%1%: must be an action list", ap);
return nullptr; }
return ap->value->to<IR::ActionList>(); }
Key getKey() const {
auto kp = properties->getProperty(TableProperties::keyPropertyName);
if (kp == nullptr)
return nullptr;
if (!kp->value->is<IR::Key>()) {
}
return nullptr;
}
inline Key getKey() const {
if (auto kp = properties->getProperty(TableProperties::keyPropertyName)) {
if (auto k = kp->value->to<IR::Key>()) return k;
::P4::error(ErrorType::ERR_INVALID, "%1%: must be a key", kp);
return nullptr; }
return kp->value->to<IR::Key>(); }
Expression getDefaultAction() const {
auto d = properties->getProperty(TableProperties::defaultActionPropertyName);
if (d == nullptr)
return nullptr;
if (!d->value->is<IR::ExpressionValue>()) {
}
return nullptr;
}
inline Expression getDefaultAction() const {
if (auto d = properties->getProperty(TableProperties::defaultActionPropertyName)) {
if (auto ev = d->value->to<IR::ExpressionValue>()) return ev->expression;
::P4::error(ErrorType::ERR_INVALID, "%1%: must be an expression", d);
return nullptr; }
return d->value->to<IR::ExpressionValue>()->expression; }
Constant getConstantProperty(cstring name) const {
}
return nullptr;
}
inline Constant getConstantProperty(cstring name) const {
if (auto d = properties->getProperty(name)) {
if (auto ev = d->value->to<IR::ExpressionValue>()) {
if (auto k = ev->expression->to<IR::Constant>()) {
return k; } }
error(ErrorType::ERR_INVALID, "%1% must be a constant numeric expression", d); }
return nullptr; }
BoolLiteral getBooleanProperty(cstring name) const {
inline BoolLiteral getBooleanProperty(cstring name) const {
if (auto d = properties->getProperty(name)) {
if (auto ev = d->value->to<IR::ExpressionValue>()) {
if (auto k = ev->expression->to<IR::BoolLiteral>()) {
return k; } }
error(ErrorType::ERR_INVALID, "%1% must be a boolean expression", d); }
return nullptr; }
Constant getSizeProperty() const {
inline Constant getSizeProperty() const {
return getConstantProperty(TableProperties::sizePropertyName);
}
EntriesList getEntries() const {
auto ep = properties->getProperty(TableProperties::entriesPropertyName);
if (ep == nullptr)
return nullptr;
if (!ep->value->is<IR::EntriesList>()) {
inline EntriesList getEntries() const {
if (auto ep = properties->getProperty(TableProperties::entriesPropertyName)) {
if (auto el = ep->value->to<IR::EntriesList>()) return el;
::P4::error(ErrorType::ERR_INVALID, "%1%: must be a list of entries", ep);
return nullptr;
}
return ep->value->to<IR::EntriesList>();
return nullptr;
}
}

Expand Down Expand Up @@ -513,17 +507,17 @@ class BlockStatement : Statement, ISimpleNamespace, IAnnotated {
void push_back(StatOrDecl st) { components.push_back(st); }
const Vector<Annotation> &getAnnotations() const override { return annotations; }
Vector<Annotation> &getAnnotations() override { return annotations; }
bool empty() const { return components.empty(); }
inline bool empty() const { return components.empty(); }
void append(StatOrDecl stmt);
BlockStatement(std::initializer_list<StatOrDecl> il) { for (auto el : il) append(el); }
inline BlockStatement(std::initializer_list<StatOrDecl> il) { for (auto el : il) append(el); }
}

class MethodCallStatement : Statement {
MethodCallExpression methodCall;
MethodCallStatement { if (!srcInfo) srcInfo = methodCall->srcInfo; }
MethodCallStatement(Util::SourceInfo si, IR::ID m, const std::initializer_list<Argument> &a)
inline MethodCallStatement { if (!srcInfo) srcInfo = methodCall->srcInfo; }
inline MethodCallStatement(Util::SourceInfo si, IR::ID m, const std::initializer_list<Argument> &a)
: Statement(si), methodCall(new MethodCallExpression(si, m, a)) {}
MethodCallStatement(Util::SourceInfo si, Expression m,
inline MethodCallStatement(Util::SourceInfo si, Expression m,
const std::initializer_list<Argument> &a)
: Statement(si), methodCall(new MethodCallExpression(si, m, a)) {}
toString{ return methodCall->toString(); }
Expand Down Expand Up @@ -644,10 +638,10 @@ abstract Block : CompileTimeValue {
virtual void dbprint_recursive(std::ostream& out) const;
/// value can be null for parameters which are optional
void setValue(Node node, CompileTimeValue value);
bool hasValue(Node node) const {
inline bool hasValue(Node node) const {
return constantValue.find(node) != constantValue.end();
}
CompileTimeValue getValue(Node node) const {
inline CompileTimeValue getValue(Node node) const {
CHECK_NULL(node);
auto it = constantValue.find(node);
BUG_CHECK(it != constantValue.end(), "%1%: No such node %2%", this, node);
Expand Down
12 changes: 2 additions & 10 deletions ir/type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ limitations under the License.
#include "frontends/common/parser_options.h"
#include "ir/configuration.h"
#include "ir/id.h"
#include "ir/ir-generated.h"
#include "ir/ir.h"
#include "ir/vector.h"
#include "lib/cstring.h"
Expand Down Expand Up @@ -172,7 +171,7 @@ const Type_State *Type_State::get(const Util::SourceInfo &si) {
}

const Type_Void *Type_Void::get() {
static const Type_Void *singleton;
static const Type_Void *singleton = nullptr;
if (!singleton) singleton = (new Type_Void());
return singleton;
}
Expand All @@ -183,7 +182,7 @@ const Type_Void *Type_Void::get(const Util::SourceInfo &si) {
}

const Type_MatchKind *Type_MatchKind::get() {
static const Type_MatchKind *singleton;
static const Type_MatchKind *singleton = nullptr;
if (!singleton) singleton = (new Type_MatchKind());
return singleton;
}
Expand All @@ -210,13 +209,6 @@ bool Type_ActionEnum::contains(cstring name) const {
return false;
}

size_t Type_MethodBase::minParameterCount() const {
size_t rv = 0;
for (auto p : *parameters)
if (!p->isOptional()) ++rv;
return rv;
}

const Type *Type_List::getP4Type() const {
auto args = new IR::Vector<Type>();
for (auto a : components) {
Expand Down
51 changes: 28 additions & 23 deletions ir/type.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class Type_Bits : Type_Base {
static Type_Bits get(const Util::SourceInfo &si, Expression expression, bool isSigned = false);
static Type_Bits get(const Util::SourceInfo &si, int sz, bool isSigned = false);
static Type_Bits get(int sz, bool isSigned = false);
cstring baseName() const { return isSigned ? "int"_cs : "bit"_cs; }
inline cstring baseName() const { return isSigned ? "int"_cs : "bit"_cs; }
int width_bits() const override { return size; }

toString{ return absl::StrCat(baseName(), "<", size, ">"); }
Expand Down Expand Up @@ -226,9 +226,9 @@ class Parameter : Declaration, IAnnotated {
optional NullOK Expression defaultValue;
const Vector<Annotation> &getAnnotations() const override { return annotations; }
Vector<Annotation> &getAnnotations() override { return annotations; }
bool hasOut() const
inline bool hasOut() const
{ return direction == IR::Direction::Out || direction == IR::Direction::InOut; }
bool isOptional() const {
inline bool isOptional() const {
return getAnnotation(Annotation::optionalAnnotation) != nullptr; }
dbprint { out << annotations << direction << (direction != IR::Direction::None ? " " : "")
<< type << ' ' << name; }
Expand All @@ -237,23 +237,23 @@ class Parameter : Declaration, IAnnotated {
class ParameterList : ISimpleNamespace {
optional inline IndexedVector<Parameter> parameters;
validate{ parameters.check_null(); }
Util::Enumerator<Parameter>* getEnumerator() const {
inline Util::Enumerator<Parameter>* getEnumerator() const {
return parameters.getEnumerator(); }
Util::Enumerator<IDeclaration>* getDeclarations() const override {
return parameters.getDeclarations(); }
size_t size() const { return parameters.size(); }
bool empty() const { return size() == 0; }
IR::Parameter getParameter(cstring name) const {
inline size_t size() const { return parameters.size(); }
inline bool empty() const { return size() == 0; }
inline IR::Parameter getParameter(cstring name) const {
return parameters.getDeclaration<Parameter>(name); }
IR::Parameter getParameter(std::string_view name) const {
inline IR::Parameter getParameter(std::string_view name) const {
return parameters.getDeclaration<Parameter>(name); }
IR::Parameter getParameter(unsigned index) const {
inline IR::Parameter getParameter(unsigned index) const {
for (auto &param : parameters)
if (0 == index--) return param;
BUG("Only %1% parameters; index #%2% requested", size(), size()+index); }
IR::IDeclaration getDeclByName(cstring name) const override { return getParameter(name); }
IR::IDeclaration getDeclByName(std::string_view name) const override { return getParameter(name); }
void push_back(const Parameter *p) { parameters.push_back(p); }
inline void push_back(const Parameter *p) { parameters.push_back(p); }
toString {
return absl::StrJoin(parameters, ", ",
[](std::string *out, const auto *p) {
Expand Down Expand Up @@ -366,13 +366,13 @@ class TypeParameters : ISimpleNamespace {
optional inline IndexedVector<Type_Var> parameters;
Util::Enumerator<IDeclaration>* getDeclarations() const override {
return parameters.getDeclarations(); }
bool empty() const { return parameters.empty(); }
size_t size() const { return parameters.size(); }
inline bool empty() const { return parameters.empty(); }
inline size_t size() const { return parameters.size(); }
IR::IDeclaration getDeclByName(cstring name) const override {
return parameters.getDeclaration(name); }
IR::IDeclaration getDeclByName(std::string_view name) const override {
return parameters.getDeclaration(name); }
void push_back(Type_Var tv) { parameters.push_back(tv); }
inline void push_back(Type_Var tv) { parameters.push_back(tv); }
validate{ parameters.check_null(); }
toString {
if (parameters.size() == 0)
Expand Down Expand Up @@ -403,9 +403,9 @@ abstract Type_StructLike : Type_Declaration, INestedNamespace, ISimpleNamespace,
Vector<Annotation> &getAnnotations() override { return annotations; }
Util::Enumerator<IDeclaration>* getDeclarations() const override {
return fields.getDeclarations(); }
StructField getField(cstring name) const {
inline StructField getField(cstring name) const {
return fields.getDeclaration<StructField>(name); }
int getFieldIndex(cstring name) const {
inline int getFieldIndex(cstring name) const {
int index_pos = 0;
for (auto f : fields) {
if (f->name == name)
Expand All @@ -420,7 +420,7 @@ abstract Type_StructLike : Type_Declaration, INestedNamespace, ISimpleNamespace,
/// Offset for all fields will be correct if:
/// - the type has only fixed width fields
/// - the type has fixed width fields with only one varbit field as a last member.
int getFieldBitOffset(cstring name) const {
inline int getFieldBitOffset(cstring name) const {
int offset = 0;
for (auto f : fields) {
if (f->name == name) {
Expand Down Expand Up @@ -467,7 +467,7 @@ class Type_HeaderUnion : Type_StructLike {
rv = std::max(rv, f->type->width_bits());
return rv; }
/// start offset of any field in a union is 0
int getFieldBitOffset(cstring name) const {
inline int getFieldBitOffset(cstring name) const {
for (auto f : fields) {
if (f->name == name) {
return 0;
Expand Down Expand Up @@ -776,9 +776,14 @@ abstract Type_MethodBase : Type, IMayBeGenericType, ISimpleNamespace {
// nullptr for constructors or functors; nullptr is not void
ParameterList parameters;

size_t maxParameterCount() const { return parameters->size(); }
size_t minParameterCount() const;
virtual TypeParameters getTypeParameters() const override { return typeParameters; }
inline size_t maxParameterCount() const { return parameters->size(); }
inline size_t minParameterCount() const {
size_t rv = 0;
for (auto p : *parameters)
if (!p->isOptional()) ++rv;
return rv;
}
TypeParameters getTypeParameters() const override { return typeParameters; }
void dbprint(std::ostream& out) const override;
toString { return "<Method>"_cs; }
const IR::Type* getP4Type() const override { return nullptr; }
Expand Down Expand Up @@ -836,9 +841,9 @@ class Method : Declaration, IAnnotated, IFunctional, ISimpleNamespace {
Type_Method type;
optional bool isAbstract = false;
optional inline Vector<Annotation> annotations;
size_t maxParameterCount() const { return type->maxParameterCount(); }
size_t minParameterCount() const { return type->minParameterCount(); }
void setAbstract() { isAbstract = true; }
inline size_t maxParameterCount() const { return type->maxParameterCount(); }
inline size_t minParameterCount() const { return type->minParameterCount(); }
inline void setAbstract() { isAbstract = true; }
const Vector<Annotation> &getAnnotations() const override { return annotations; }
Vector<Annotation> &getAnnotations() override { return annotations; }
ParameterList getParameters() const override { return type->parameters; }
Expand Down
2 changes: 1 addition & 1 deletion lib/error_reporter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ class ErrorReporter {
std::string message;
if (!absl::FormatUntyped(&message, absl::UntypedFormatSpec(fmt),
{absl::FormatArg(args)...})) {
BUG("Failed to format string");
BUG("Failed to format string %s", fmt);
}

emit_message(ParserErrorMessage(Util::SourceInfo(sources, position), std::move(message)));
Expand Down
2 changes: 1 addition & 1 deletion tools/ir-generator/ir-generator.ypp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ method
{ ($$ = $<irMethod>5)->body = $10;
if (!$10 || $10.startsWith("=")) {
if (!$$->inImpl)
yyerror("inline method %s with no body", $3);
yyerror("inline method %v with no body", $3);
$$->inImpl = false; }
$$->isUser = true;
$$->isConst = $8;
Expand Down
2 changes: 1 addition & 1 deletion tools/ir-generator/irclass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ int IrClass::generateConstructor(const ctor_args_t &arglist, const IrMethod *use
}

if (kind == NodeKind::Abstract) ctor->access = IrElement::Protected;
ctor->inImpl = true;
ctor->inImpl = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will have major effect on the size of the ir-generated.h header. Is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, currently all simple the constructors are outlined. This case covers those that are not in defined explicitly and just initialize the fields. As a result lots of function calls are emitted as compiler is unable to inline those obviously.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We have some deeply nested inheritance trees. Wouldn't this lead to a blowup if every constructor can be inlined?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We are also adding ~4k LoC to the header in the worst case (with Tofino)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have some deeply nested inheritance trees. Wouldn't this lead to a blowup if every constructor can be inlined?

Not quite. Remember that inlining decision is done by compiler. Not every function declared inline will be inlined. Also, explicitly defined constructors w/o inline will still be outlined, there is a way to control that. Implicit ones are the ones that initialize the fields just forwarding the arguments.

elements.push_back(ctor);
return optargs;
}
Expand Down
Loading