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

[NFC] Eliminate the majority of dynamic_cast usage in the codebase in favor of ICastable interface #4382

Merged
merged 4 commits into from
Feb 1, 2024
Merged

[NFC] Eliminate the majority of dynamic_cast usage in the codebase in favor of ICastable interface #4382

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fee0cf6
Eliminate the majority of `dynamic_cast` usage in the codebase in fav…
asl Jan 30, 2024
b1ff849
Produce slightly better error message in case of failed cast
asl Jan 30, 2024
2298ff1
Apply suggestions from code review
asl Jan 31, 2024
f5dd68a
Apply suggestions from code review
asl Jan 31, 2024
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
13 changes: 6 additions & 7 deletions backends/bmv2/simple_switch/simpleSwitch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,24 +84,23 @@ class SimpleSwitchExpressionConverter : public ExpressionConverter {
}

bool isStandardMetadataParameter(const IR::Parameter *param) {
auto st = dynamic_cast<V1ProgramStructure *>(structure);
auto params = st->parser->getApplyParameters();
auto params = structure->parser->getApplyParameters();
if (params->size() != 4) {
modelError("%1%: Expected 4 parameter for parser", st->parser);
modelError("%1%: Expected 4 parameter for parser", structure->parser);
return false;
}
if (params->parameters.at(3) == param) return true;

params = st->ingress->getApplyParameters();
params = structure->ingress->getApplyParameters();
if (params->size() != 3) {
modelError("%1%: Expected 3 parameter for ingress", st->ingress);
modelError("%1%: Expected 3 parameter for ingress", structure->ingress);
return false;
}
if (params->parameters.at(2) == param) return true;

params = st->egress->getApplyParameters();
params = structure->egress->getApplyParameters();
if (params->size() != 3) {
modelError("%1%: Expected 3 parameter for egress", st->egress);
modelError("%1%: Expected 3 parameter for egress", structure->egress);
return false;
}
if (params->parameters.at(2) == param) return true;
Expand Down
4 changes: 2 additions & 2 deletions backends/ebpf/ebpfControl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ bool ControlBodyTranslator::preorder(const IR::MethodCallExpression *expression)

void ControlBodyTranslator::compileEmitField(const IR::Expression *expr, cstring field,
unsigned alignment, EBPFType *type) {
unsigned widthToEmit = dynamic_cast<IHasWidth *>(type)->widthInBits();
unsigned widthToEmit = type->as<IHasWidth>().widthInBits();
vlstill marked this conversation as resolved.
Show resolved Hide resolved
cstring swap = "";
if (widthToEmit == 16)
swap = "htons";
Expand Down Expand Up @@ -254,7 +254,7 @@ void ControlBodyTranslator::compileEmit(const IR::Vector<IR::Argument> *args) {
for (auto f : ht->fields) {
auto ftype = typeMap->getType(f);
auto etype = EBPFTypeFactory::instance->create(ftype);
auto et = dynamic_cast<IHasWidth *>(etype);
auto et = etype->to<IHasWidth>();
if (et == nullptr) {
::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,
"Only headers with fixed widths supported %1%", f);
Expand Down
4 changes: 2 additions & 2 deletions backends/ebpf/ebpfDeparser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ void DeparserHdrEmitTranslator::processMethod(const P4::ExternMethod *method) {
for (auto f : headerToEmit->fields) {
auto ftype = deparser->program->typeMap->getType(f);
auto etype = EBPFTypeFactory::instance->create(ftype);
auto et = dynamic_cast<EBPF::IHasWidth *>(etype);
auto et = etype->to<IHasWidth>();
if (et == nullptr) {
::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,
"Only headers with fixed widths supported %1%", f);
Expand All @@ -171,7 +171,7 @@ void DeparserHdrEmitTranslator::emitField(CodeBuilder *builder, cstring field,
EBPF::EBPFType *type) {
auto program = deparser->program;

auto et = dynamic_cast<EBPF::IHasWidth *>(type);
auto et = type->to<IHasWidth>();
if (et == nullptr) {
::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,
"Only headers with fixed widths supported %1%", hdrExpr);
Expand Down
4 changes: 2 additions & 2 deletions backends/ebpf/ebpfParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ bool StateTranslationVisitor::preorder(const IR::SelectCase *selectCase) {
void StateTranslationVisitor::compileExtractField(const IR::Expression *expr,
const IR::StructField *field, unsigned alignment,
EBPFType *type) {
unsigned widthToExtract = dynamic_cast<IHasWidth *>(type)->widthInBits();
unsigned widthToExtract = type->as<IHasWidth>().widthInBits();
auto program = state->parser->program;
cstring msgStr;
cstring fieldName = field->name.name;
Expand Down Expand Up @@ -470,7 +470,7 @@ void StateTranslationVisitor::compileExtract(const IR::Expression *destination)
for (auto f : ht->fields) {
auto ftype = state->parser->typeMap->getType(f);
auto etype = EBPFTypeFactory::instance->create(ftype);
auto et = dynamic_cast<IHasWidth *>(etype);
auto et = etype->to<IHasWidth>();
if (et == nullptr) {
::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,
"Only headers with fixed widths supported %1%", f);
Expand Down
2 changes: 1 addition & 1 deletion backends/ebpf/ebpfTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ void EBPFTable::emitAction(CodeBuilder *builder, cstring valueName, cstring acti
builder->target->emitTraceMessage(builder, msgStr.c_str());
for (auto param : *(action->parameters)) {
auto etype = EBPFTypeFactory::instance->create(param->type);
unsigned width = dynamic_cast<IHasWidth *>(etype)->widthInBits();
unsigned width = etype->as<IHasWidth>().widthInBits();

if (width <= 64) {
convStr = Util::printf_format("(unsigned long long) (%s->u.%s.%s)", valueName, name,
Expand Down
16 changes: 9 additions & 7 deletions backends/ebpf/ebpfType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,11 @@ void EBPFStackType::emitInitializer(CodeBuilder *builder) {
builder->append(" }");
}

unsigned EBPFStackType::widthInBits() { return size * elementType->to<IHasWidth>()->widthInBits(); }
unsigned EBPFStackType::widthInBits() const {
return size * elementType->to<IHasWidth>()->widthInBits();
}

unsigned EBPFStackType::implementationWidthInBits() {
unsigned EBPFStackType::implementationWidthInBits() const {
return size * elementType->to<IHasWidth>()->implementationWidthInBits();
}

Expand Down Expand Up @@ -177,7 +179,7 @@ EBPFStructType::EBPFStructType(const IR::Type_StructLike *strct) : EBPFType(strc

for (auto f : strct->fields) {
auto type = EBPFTypeFactory::instance->create(f->type);
auto wt = dynamic_cast<IHasWidth *>(type);
auto wt = type->to<IHasWidth>();
if (wt == nullptr) {
::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET, "EBPF: Unsupported type in struct: %s",
f->type);
Expand Down Expand Up @@ -274,17 +276,17 @@ void EBPFTypeName::emitInitializer(CodeBuilder *builder) {
if (canonical != nullptr) canonical->emitInitializer(builder);
}

unsigned EBPFTypeName::widthInBits() {
auto wt = dynamic_cast<IHasWidth *>(canonical);
unsigned EBPFTypeName::widthInBits() const {
auto wt = canonical->to<IHasWidth>();
if (wt == nullptr) {
::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET, "Type %1% does not have a fixed witdh", type);
return 0;
}
return wt->widthInBits();
}

unsigned EBPFTypeName::implementationWidthInBits() {
auto wt = dynamic_cast<IHasWidth *>(canonical);
unsigned EBPFTypeName::implementationWidthInBits() const {
auto wt = canonical->to<IHasWidth>();
if (wt == nullptr) {
::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET, "Type %1% does not have a fixed witdh", type);
return 0;
Expand Down
40 changes: 16 additions & 24 deletions backends/ebpf/ebpfType.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,16 @@ class EBPFType : public EBPFObject {
virtual void declareArray(CodeBuilder * /*builder*/, cstring /*id*/, unsigned /*size*/) {
BUG("%1%: unsupported array", type);
}
template <typename T>
bool is() const {
return dynamic_cast<const T *>(this) != nullptr;
}
template <typename T>
T *to() {
return dynamic_cast<T *>(this);
}
};

class IHasWidth {
class IHasWidth : public ICastable {
public:
virtual ~IHasWidth() {}
// P4 width
virtual unsigned widthInBits() = 0;
virtual unsigned widthInBits() const = 0;
// Width in the target implementation.
// Currently a multiple of 8.
virtual unsigned implementationWidthInBits() = 0;
virtual unsigned implementationWidthInBits() const = 0;
};

class EBPFTypeFactory {
Expand All @@ -78,8 +70,8 @@ class EBPFBoolType : public EBPFType, public IHasWidth {
void declare(CodeBuilder *builder, cstring id, bool asPointer) override;
void declareInit(CodeBuilder *builder, cstring id, bool asPointer) override;
void emitInitializer(CodeBuilder *builder) override { builder->append("0"); }
unsigned widthInBits() override { return 1; }
unsigned implementationWidthInBits() override { return 8; }
unsigned widthInBits() const override { return 1; }
unsigned implementationWidthInBits() const override { return 8; }
};

class EBPFStackType : public EBPFType, public IHasWidth {
Expand All @@ -97,8 +89,8 @@ class EBPFStackType : public EBPFType, public IHasWidth {
void declare(CodeBuilder *builder, cstring id, bool asPointer) override;
void declareInit(CodeBuilder *builder, cstring id, bool asPointer) override;
void emitInitializer(CodeBuilder *builder) override;
unsigned widthInBits() override;
unsigned implementationWidthInBits() override;
unsigned widthInBits() const override;
unsigned implementationWidthInBits() const override;
};

class EBPFScalarType : public EBPFType, public IHasWidth {
Expand All @@ -115,8 +107,8 @@ class EBPFScalarType : public EBPFType, public IHasWidth {
void declare(CodeBuilder *builder, cstring id, bool asPointer) override;
void declareInit(CodeBuilder *builder, cstring id, bool asPointer) override;
void emitInitializer(CodeBuilder *builder) override;
unsigned widthInBits() override { return width; }
unsigned implementationWidthInBits() override { return bytesRequired() * 8; }
unsigned widthInBits() const override { return width; }
unsigned implementationWidthInBits() const override { return bytesRequired() * 8; }
// True if this width is small enough to store in a machine scalar
static bool generatesScalar(unsigned width) { return width <= 64; }
};
Expand All @@ -133,13 +125,13 @@ class EBPFTypeName : public EBPFType, public IHasWidth {
void declare(CodeBuilder *builder, cstring id, bool asPointer) override;
void declareInit(CodeBuilder *builder, cstring id, bool asPointer) override;
void emitInitializer(CodeBuilder *builder) override;
unsigned widthInBits() override;
unsigned implementationWidthInBits() override;
unsigned widthInBits() const override;
unsigned implementationWidthInBits() const override;
void declareArray(CodeBuilder *builder, cstring id, unsigned size) override;

template <typename T>
bool canonicalTypeIs() const {
return dynamic_cast<const T *>(this->canonical) != nullptr;
return this->canonical->is<T>();
}
};

Expand All @@ -166,8 +158,8 @@ class EBPFStructType : public EBPFType, public IHasWidth {
void declare(CodeBuilder *builder, cstring id, bool asPointer) override;
void declareInit(CodeBuilder *builder, cstring id, bool asPointer) override;
void emitInitializer(CodeBuilder *builder) override;
unsigned widthInBits() override { return width; }
unsigned implementationWidthInBits() override { return implWidth; }
unsigned widthInBits() const override { return width; }
unsigned implementationWidthInBits() const override { return implWidth; }
void emit(CodeBuilder *builder) override;
void declareArray(CodeBuilder *builder, cstring id, unsigned size) override;
};
Expand All @@ -179,8 +171,8 @@ class EBPFEnumType : public EBPFType, public EBPF::IHasWidth {
void declare(CodeBuilder *builder, cstring id, bool asPointer) override;
void declareInit(CodeBuilder *builder, cstring id, bool asPointer) override;
void emitInitializer(CodeBuilder *builder) override { builder->append("0"); }
unsigned widthInBits() override { return 32; }
unsigned implementationWidthInBits() override { return 32; }
unsigned widthInBits() const override { return 32; }
unsigned implementationWidthInBits() const override { return 32; }
const IR::Type_Enum *getType() const { return type->to<IR::Type_Enum>(); }
};

Expand Down
2 changes: 1 addition & 1 deletion backends/ebpf/psa/ebpfPsaDeparser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ void TCIngressDeparserPSA::emitPreDeparser(CodeBuilder *builder) {
"skipping deparser..");
builder->emitIndent();
CHECK_NULL(program);
auto pipeline = dynamic_cast<const EBPFPipeline *>(program);
auto pipeline = program->checkedTo<EBPFPipeline>();
builder->appendFormat("%s->packet_path = RESUBMIT;", pipeline->compilerGlobalMetadata);
builder->newline();
builder->emitIndent();
Expand Down
2 changes: 1 addition & 1 deletion backends/ebpf/psa/externs/ebpfPsaCounter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ void EBPFCounterPSA::emitDirectMethodInvocation(CodeBuilder *builder,
BUG_CHECK(method->expr->arguments->size() == 0, "Expected no arguments for %1%", method->expr);

cstring target = valuePtr + "->" + instanceName;
auto pipeline = dynamic_cast<const EBPFPipeline *>(program);
auto pipeline = program->to<EBPFPipeline>();
CHECK_NULL(pipeline);

cstring msgStr =
Expand Down
4 changes: 2 additions & 2 deletions backends/ebpf/psa/externs/ebpfPsaMeter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ void EBPFMeterPSA::emitInstance(CodeBuilder *builder) const {

void EBPFMeterPSA::emitExecute(CodeBuilder *builder, const P4::ExternMethod *method,
ControlBodyTranslatorPSA *translator) const {
auto pipeline = dynamic_cast<const EBPFPipeline *>(program);
auto pipeline = program->to<EBPFPipeline>();
CHECK_NULL(pipeline);

cstring functionNameSuffix;
Expand Down Expand Up @@ -202,7 +202,7 @@ void EBPFMeterPSA::emitIndex(CodeBuilder *builder, const P4::ExternMethod *metho

void EBPFMeterPSA::emitDirectExecute(CodeBuilder *builder, const P4::ExternMethod *method,
cstring valuePtr) const {
auto pipeline = dynamic_cast<const EBPFPipeline *>(program);
auto pipeline = program->to<EBPFPipeline>();
CHECK_NULL(pipeline);

cstring functionNameSuffix;
Expand Down
2 changes: 1 addition & 1 deletion backends/ebpf/psa/externs/ebpfPsaRegister.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ EBPFRegisterPSA::EBPFRegisterPSA(const EBPFProgram *program, cstring instanceNam

bool EBPFRegisterPSA::shouldUseArrayMap() {
CHECK_NULL(this->keyType);
if (auto wt = dynamic_cast<IHasWidth *>(this->keyType)) {
if (auto wt = this->keyType->to<IHasWidth>()) {
unsigned keyWidth = wt->widthInBits();
// For keys <= 32 bit register is based on array map,
// otherwise we use hash map
Expand Down
2 changes: 1 addition & 1 deletion backends/p4tools/common/core/abstract_execution_state.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ const IR::Expression *AbstractExecutionState::convertToComplexExpression(
IR::Vector<IR::Expression> components;
const auto *elementType = resolveType(ts->elementType);
for (size_t idx = 0; idx < ts->getSize(); idx++) {
auto ref = new IR::ArrayIndex(elementType, parent, new IR::Constant(idx));
auto ref = new IR::ArrayIndex(elementType, parent, new IR::Constant(uint64_t(idx)));
vlstill marked this conversation as resolved.
Show resolved Hide resolved
asl marked this conversation as resolved.
Show resolved Hide resolved
if (elementType->is<IR::Type_StructLike>() || elementType->to<IR::Type_Stack>()) {
components.push_back(convertToComplexExpression(ref));
} else {
Expand Down
Loading
Loading