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

Fix many Tofino warnings. #5007

Merged
merged 1 commit into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 0 additions & 1 deletion backends/tofino/bf-p4c/arch/fromv1.0/stateful_alu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,6 @@ class CreateSaluApplyFunction : public Inspector {
int expr_idx[NUM_EXPR_INDEX_T] = {0};
Util::SourceInfo expr_src_info[NUM_EXPR_INDEX_T];
Util::SourceInfo expr_pred_src_info[NUM_EXPR_INDEX_T];
const IR::Statement *output = nullptr;
const Util::SourceInfo *applyLoc = nullptr;
enum expr_index_t { LO1, LO2, HI1, HI2, OUT, UNUSED } expr_index = UNUSED;
bool saturating = false;
Expand Down
8 changes: 5 additions & 3 deletions backends/tofino/bf-p4c/arch/fromv1.0/v1_converters.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,17 @@ class ControlConverter : public Transform {
CHECK_NULL(structure);
}

const IR::Node *postorder(IR::Declaration_Instance *node) {
const IR::Node *postorder(IR::Declaration_Instance *node) override {
return substitute<IR::Declaration_Instance>(node);
}

const IR::Node *postorder(IR::MethodCallStatement *node) {
const IR::Node *postorder(IR::MethodCallStatement *node) override {
return substitute<IR::MethodCallStatement>(node);
}

const IR::Node *postorder(IR::Property *node) { return substitute<IR::Property>(node); }
const IR::Node *postorder(IR::Property *node) override {
return substitute<IR::Property>(node);
}

const IR::Node *postorder(IR::BFN::TnaControl *node) override;

Expand Down
2 changes: 1 addition & 1 deletion backends/tofino/bf-p4c/arch/psa/psa.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ class TranslateProgram : public Inspector {
return;
}

void postorder(const IR::ParserState *state) {
void postorder(const IR::ParserState *state) override {
auto ctxt = findContext<IR::P4Parser>();
CHECK_NULL(ctxt);
for (auto stmt : state->components) {
Expand Down
2 changes: 1 addition & 1 deletion backends/tofino/bf-p4c/common/field_defuse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ static inline auto getParserRangeDefMatcher(const PhvInfo &phv, const PHV::Field
le_bitrange range = bits ? *bits : StartLen(0, f->size);
// note that we need to capture f (pointer) and range by value to avoid dangling reference,
// but phv by reference (it is large and we got it by reference)
return [&phv, range, f](const FieldDefUse::locpair &lp) {
return [&phv, range](const FieldDefUse::locpair &lp) {
le_bitrange rng;
if (!(lp.first->is<IR::BFN::ParserState>() || lp.first->is<IR::BFN::Parser>()))
return false;
Expand Down
21 changes: 10 additions & 11 deletions backends/tofino/bf-p4c/control-plane/bfruntime_arch_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ struct CounterlikeTraits<::BFN::CounterExtern<BFN::Arch::TNA>> {
/// CounterlikeTraits specialization for BFN::CounterExtern for PSA
template <>
struct CounterlikeTraits<::BFN::CounterExtern<BFN::Arch::PSA>> {
static const cstring name() { return "counter"_cs; }
static const cstring directPropertyName() { return "psa_direct_counter"_cs; }
static const cstring typeName() { return "Counter"_cs; }
static const cstring directTypeName() { return "DirectCounter"_cs; }
static const cstring sizeParamName() { return "n_counters"_cs; }
static cstring name() { return "counter"_cs; }
static cstring directPropertyName() { return "psa_direct_counter"_cs; }
static cstring typeName() { return "Counter"_cs; }
static cstring directTypeName() { return "DirectCounter"_cs; }
static cstring sizeParamName() { return "n_counters"_cs; }
static ::barefoot::CounterSpec::Unit mapUnitName(const cstring name) {
using ::barefoot::CounterSpec;
if (name == "PACKETS")
Expand Down Expand Up @@ -519,8 +519,7 @@ struct SnapshotFieldInfo {
uint32_t id;
int32_t bitwidth;
bool operator==(const SnapshotFieldInfo &s) const {
if (s.name == name && s.id == id && s.bitwidth == bitwidth) return true;
return false;
return s.name == name && s.id == id && s.bitwidth == bitwidth;
}
bool operator<(const SnapshotFieldInfo &s) const { return (id < s.id); }
};
Expand Down Expand Up @@ -2392,7 +2391,7 @@ class BFRuntimeArchHandlerPSA final : public BFRuntimeArchHandlerCommon<Arch::PS
implementationString = "psa_implementation"_cs;
}

void postCollect(const P4RuntimeSymbolTableIface &symbols) {
void postCollect(const P4RuntimeSymbolTableIface &symbols) override {
(void)symbols;
// analyze action profiles / selectors and build a mapping from action
// profile / selector name to the set of tables referencing them
Expand Down Expand Up @@ -2439,15 +2438,15 @@ class BFRuntimeArchHandlerPSA final : public BFRuntimeArchHandlerCommon<Arch::PS
collectExternInstanceCommon(symbols, externBlock);
}

void collectExternFunction(P4RuntimeSymbolTableIface *, const P4::ExternFunction *) {}
void collectExternFunction(P4RuntimeSymbolTableIface *, const P4::ExternFunction *) override {}

void addExternInstance(const P4RuntimeSymbolTableIface &symbols, p4configv1::P4Info *p4info,
const IR::ExternBlock *externBlock) {
const IR::ExternBlock *externBlock) override {
addExternInstanceCommon(symbols, p4info, externBlock, defaultPipeName);
}

void addExternFunction(const P4RuntimeSymbolTableIface &, p4configv1::P4Info *,
const P4::ExternFunction *) {}
const P4::ExternFunction *) override {}
};

/// The architecture handler builder implementation for Tofino.
Expand Down
6 changes: 3 additions & 3 deletions backends/tofino/bf-p4c/logging/constrained_fields.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class ConstrainedField : public LoggableEntity {
bool sameContainerGroup = false;

public:
ConstrainedField() {}
ConstrainedField() = default;
explicit ConstrainedField(const cstring &name);

const cstring &getName() const { return name; }
Expand All @@ -116,7 +116,7 @@ class ConstrainedField : public LoggableEntity {
const Constraints::DigestConstraint &getDigest() const { return digest; }

void setContainerSize(const Constraints::ContainerSizeConstraint &containerSize);
const Constraints::ContainerSizeConstraint getContainerSize() const { return containerSize; }
Constraints::ContainerSizeConstraint getContainerSize() const { return containerSize; }

void setBottomBits(bool b);
bool hasBottomBits() const { return deparsedBottomBits; }
Expand All @@ -134,7 +134,7 @@ class ConstrainedField : public LoggableEntity {
bool hasNoHoles() const { return noHoles; }

void setSameContainerGroup(bool b);
bool hasSameContainerGroup() { return sameContainerGroup; }
bool hasSameContainerGroup() const { return sameContainerGroup; }
};

typedef std::map<cstring, ConstrainedField> ConstrainedFieldMap;
Expand Down
6 changes: 2 additions & 4 deletions backends/tofino/bf-p4c/logging/resources_clot.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,11 @@
#ifndef _BACKENDS_TOFINO_BF_P4C_LOGGING_RESOURCES_CLOT_H_
#define _BACKENDS_TOFINO_BF_P4C_LOGGING_RESOURCES_CLOT_H_

/* clang-format off */
#include <map>
#include <vector>
#include "ir/ir.h"

#include "backends/tofino/bf-p4c/logging/resources_schema.h"
/* clang-format on */
#include "ir/ir.h"

using Logging::Resources_Schema_Logger;
class ClotInfo; // Forward declaration
Expand All @@ -50,7 +48,7 @@ class ClotResourcesLogging : public ParserInspector {

std::vector<ClotUsage *> &getUsageData(gress_t gress, unsigned tag);

bool preorder(const IR::BFN::LoweredParserState *state);
bool preorder(const IR::BFN::LoweredParserState *state) override;
void end_apply() override;

void collectClotUsages(const IR::BFN::LoweredParserMatch *match,
Expand Down
1 change: 0 additions & 1 deletion backends/tofino/bf-p4c/mau/action_analysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,6 @@ bool ActionAnalysis::init_simple_alignment(const ActionParam &read, ContainerAct

if (read.type == ActionParam::ACTIONDATA) {
le_bitrange read_bits;
cstring key;
if (cont_action.is_deposit_field_variant || cont_action.convert_instr_to_deposit_field ||
cont_action.convert_instr_to_byte_rotate_merge || cont_action.name == "set") {
// if the instruction is or could be a deposit field or byte rotate, then we don't
Expand Down
2 changes: 0 additions & 2 deletions backends/tofino/bf-p4c/mau/action_format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2101,7 +2101,6 @@ const ALUParameter *Format::Use::find_param_alloc(UniqueLocationKey &key,
Log::TempIndent indent;
LOG3(" Finding ALU Param with container " << indent << indent << key.container
<< " for action " << key.action_name);
bool container_match = false;
for (auto alu_position : action_alu_positions) {
LOG3(alu_position);
if (alu_position.alu_op->container() != key.container) continue;
Expand All @@ -2113,7 +2112,6 @@ const ALUParameter *Format::Use::find_param_alloc(UniqueLocationKey &key,
"in container %s in action %s",
key.container, key.action_name);
rv = loc;
container_match |= alu_position.alu_op->container() == key.container;
if (alu_pos_p) *alu_pos_p = new ALUPosition(alu_position);
}
if (!rv) {
Expand Down
2 changes: 0 additions & 2 deletions backends/tofino/bf-p4c/mau/determine_power_usage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,8 @@ void DeterminePowerUsage::postorder(const IR::MAU::Table *t) {
} else if (mem.type == Memories::Use::EXACT) {
// One unit of depth in each way is accessed on a table read.
// Total RAMs accessed will then be the sum of the widths of ways.
int num_ways = 0;
for (auto way : t->ways) {
// LOG4(" exact match width = " << way.width);
++num_ways;
match_table.ram_read += way.width;
}
mau_features_->has_exact_[t->gress][t->stage()] = true;
Expand Down
4 changes: 2 additions & 2 deletions backends/tofino/bf-p4c/mau/gateway.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1287,7 +1287,7 @@ bool BuildGatewayMatch::preorder(const IR::Expression *e) {
const IR::Expression *tmp;
BUG_CHECK(orig_mask == donemask, "failed to match all bits of %s",
(tmp = findContext<IR::Operation::Relation>()) ? tmp : e);
BUG_CHECK((mask ^ upper_bit_mask) << shift == 0, "Didn't cover all bytes");
BUG_CHECK(((mask ^ upper_bit_mask) << shift) == 0, "Didn't cover all bytes");
match_field = {};
}
return false;
Expand Down Expand Up @@ -1375,7 +1375,7 @@ void BuildGatewayMatch::constant(big_int c) {
}
LOG6(" match now " << match);
}
BUG_CHECK((mask ^ upper_bits_mask) << shift == 0, "Didn't cover all bytes");
BUG_CHECK(((mask ^ upper_bits_mask) << shift) == 0, "Didn't cover all bytes");
match_field = {};
} else {
BUG("Invalid context for constant in BuildGatewayMatch");
Expand Down
4 changes: 2 additions & 2 deletions backends/tofino/bf-p4c/mau/instruction_selection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3197,12 +3197,12 @@ class ExpandInstructions : public MauTransform {
}
}

IR::Node *preorder(IR::MAU::Action *act) {
IR::Node *preorder(IR::MAU::Action *act) override {
check_action_data_bus_params(act);
return act;
}

IR::Node *postorder(IR::Expression *expr) {
IR::Node *postorder(IR::Expression *expr) override {
auto orig_expr = getOriginal<IR::Expression>();
BUG_CHECK(orig_expr, "Couldn't find original IR node");

Expand Down
4 changes: 2 additions & 2 deletions backends/tofino/bf-p4c/mau/jbay_next_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ class JbayNextTable::FindNextTableUse : public MauTableInspector {
return r1.second > r2.first && r2.second > r1.first;
}

profile_t init_apply(const IR::Node *root) {
profile_t init_apply(const IR::Node *root) override {
self.use_next_table.clear();
tables.clear();
return MauTableInspector::init_apply(root);
Expand All @@ -360,7 +360,7 @@ class JbayNextTable::FindNextTableUse : public MauTableInspector {
// containing seq, if that was useful. Probably almost never useful?
if (!t->next.empty()) tables.push_back(t);
}
void postorder(const IR::BFN::Pipe *) {
void postorder(const IR::BFN::Pipe *) override {
std::stable_sort(tables.begin(), tables.end(),
[this](const IR::MAU::Table *a, const IR::MAU::Table *b) {
return self.control_dep.paths(a) > self.control_dep.paths(b);
Expand Down
12 changes: 6 additions & 6 deletions backends/tofino/bf-p4c/mau/stateful_alu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1581,12 +1581,12 @@ bool CreateSaluInstruction::isComplexInstruction(const IR::Operation_Binary *op)
// IR::L* operators (LAnd, LOr, etc.) are working with predicates

bool ret = false;
for (auto oper : {op->left, op->right}) {
ret |= oper->is<IR::Add>() | oper->is<IR::AddSat>();
ret |= oper->is<IR::Sub>() | oper->is<IR::SubSat>();
ret |= oper->is<IR::BAnd>() | oper->is<IR::BOr>() | oper->is<IR::BXor>();
ret |= oper->is<IR::Div>() | oper->is<IR::Mod>();
ret |= oper->is<IR::Neg>();
for (const auto *oper : {op->left, op->right}) {
ret = ret || oper->is<IR::Add>() || oper->is<IR::AddSat>();
ret = ret || oper->is<IR::Sub>() || oper->is<IR::SubSat>();
ret = ret || oper->is<IR::BAnd>() || oper->is<IR::BOr>() || oper->is<IR::BXor>();
ret = ret || oper->is<IR::Div>() || oper->is<IR::Mod>();
ret = ret || oper->is<IR::Neg>();
}

return ret;
Expand Down
4 changes: 1 addition & 3 deletions backends/tofino/bf-p4c/mau/table_placement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4881,7 +4881,7 @@ std::ostream &operator<<(std::ostream &out, TablePlacement::choice_t choice) {
"control dom set has more placeable tables",
"average chain length of control dom set",
"default choice"};
if (choice < sizeof(choice_names) / sizeof(choice_names[0])) {
if (static_cast<uint64_t>(choice) < sizeof(choice_names) / sizeof(choice_names[0])) {
out << choice_names[choice];
} else {
out << "unknown choice <0x" << hex(choice) << ">";
Expand Down Expand Up @@ -5476,7 +5476,6 @@ IR::Node *TransformTables::preorder(IR::MAU::Table *tbl) {
return tbl;
}
int stage_table = 0;
int prev_entries = 0;
IR::Vector<IR::MAU::Table> *rv = new IR::Vector<IR::MAU::Table>;
IR::MAU::Table *prev = 0;
IR::MAU::Table *atcam_last = nullptr;
Expand Down Expand Up @@ -5679,7 +5678,6 @@ IR::Node *TransformTables::preorder(IR::MAU::Table *tbl) {
stage_table++;
prev = table_part;
if (atcam_last) prev = atcam_last;
prev_entries += pl->entries;
}
BUG_CHECK(!rv->empty(), "Failed to place any stage tables for %s", tbl->name);
return rv;
Expand Down
4 changes: 0 additions & 4 deletions backends/tofino/bf-p4c/mau/tofino/input_xbar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2098,7 +2098,6 @@ bool IXBar::allocHashWay(const IR::MAU::Table *tbl, const LayoutOption *layout_o
}
// Calculation of the separate select bits among many stages
unsigned free_bits = 0;
unsigned used_bits = 0;

for (int bit = 0; bit < IXBar::get_hash_single_bits(); bit++) {
if (!(hash_single_bit_inuse[bit] & hf_hash_table_input)) {
Expand All @@ -2107,7 +2106,6 @@ bool IXBar::allocHashWay(const IR::MAU::Table *tbl, const LayoutOption *layout_o
}
for (auto &way_use : alloc.way_use) {
BUG_CHECK(way_use.select.lo == RAM_SELECT_BIT_START, "invalid select range for tofino");
used_bits |= way_use.select_mask;
}

if (way_bits == 0) {
Expand Down Expand Up @@ -3796,7 +3794,6 @@ void IXBar::XBarHashDist::immediate_inputs() {
auto hash = pos.second->to<ActionData::Hash>();
if (hash == nullptr) continue;
le_bitrange immed_range = {pos.first, pos.first + hash->size() - 1};
int bits_seen = 0;
for (int i = 0; i < 2; i++) {
le_bitrange immed_impact = {i * HASH_DIST_BITS, (i + 1) * HASH_DIST_BITS - 1};
auto boost_sl = toClosedRange<RangeUnit::Bit, Endian::Little>(
Expand All @@ -3810,7 +3807,6 @@ void IXBar::XBarHashDist::immediate_inputs() {
func->slice(hash_range);
HashDistDest_t dest = static_cast<HashDistDest_t>(i);
alloc_reqs.emplace_back(func, hash_dist_range, dest, 0);
bits_seen += overlap.size();
}
}
}
Expand Down
12 changes: 3 additions & 9 deletions backends/tofino/bf-p4c/mau/tofino/memories.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3530,11 +3530,9 @@ bool Memories::allocate_all_swbox_users() {
}

if (!action_bus_users.empty() || !synth_bus_users.empty()) {
int act_unused = 0;
for (auto abu : action_bus_users) act_unused += abu->left_to_place();
for (auto abu : action_bus_users) abu->left_to_place();

int sup_unused = 0;
for (auto sbu : synth_bus_users) sup_unused += sbu->left_to_place();
for (auto sbu : synth_bus_users) sbu->left_to_place();

failure_reason = "allocate_all_swbox_users failed"_cs;
LOG4(failure_reason);
Expand Down Expand Up @@ -4034,10 +4032,9 @@ bool Memories::allocate_all_gw() {
}
}

int search_bus_free = 0;
for (int i = 0; i < SRAM_ROWS; i++) {
for (int j = 0; j < 2; j++) {
if (sram_search_bus[i][j].free()) search_bus_free++;
sram_search_bus[i][j].free();
}
}

Expand All @@ -4061,7 +4058,6 @@ bool Memories::allocate_all_no_match_miss() {
// so this is essentially what I'm doing. More discussion is needed with the driver
// team in order to determine if this is correct, or if this has to go through ternary and
// tind tables
size_t no_match_tables_allocated = 0;
for (auto *ta : no_match_miss_tables) {
for (auto u_id : ta->allocation_units(nullptr, false, UniqueAttachedId::TIND_PP)) {
auto &alloc = (*ta->memuse)[u_id];
Expand All @@ -4086,8 +4082,6 @@ bool Memories::allocate_all_no_match_miss() {
failure_reason = "failed to place no match miss "_cs + u_id.build_name();
LOG4(failure_reason);
return false;
} else {
no_match_tables_allocated++;
}
}
}
Expand Down
Loading
Loading