From 5fda7d293eaf99c18702e3b154b325da0bfe0e07 Mon Sep 17 00:00:00 2001 From: Chris Dodd Date: Sun, 7 Jul 2024 14:43:07 +1200 Subject: [PATCH] Fix loop unrolling bugs related to issue4739 (#4783) * Fix loop unrolling bugs related to issue4739 - fix FindUninitialized in simplifyDefUse.cpp to correctly get live state for for loop bodies, so as to not dead-code eliminate things unused after the loop exit - fix midend/def_use.cpp ComputeDefUse to visit assignments right then left to get correct info for assignments that read the old value of the lhs - fix loopUnrolling.cpp UnrollLoops to not unroll loops where the index is modified in the loop body in addition to the loop increment. - more logging in ComputeDefUse to see help debugging Signed-off-by: Chris Dodd * XFAILS for testgen for new test, as it barfs on loops Signed-off-by: Chris Dodd * Rearrange and simplify the code for determining if a loop is unrollable - just need to look at def-use chains for the index var, so actually will unroll a few more cases. Signed-off-by: Chris Dodd --------- Signed-off-by: Chris Dodd --- .../targets/bmv2/test/BMV2MetadataXfail.cmake | 6 ++ .../targets/bmv2/test/BMV2PTFXfail.cmake | 6 ++ .../bmv2/test/BMV2ProtobufIrXfail.cmake | 6 ++ .../targets/bmv2/test/BMV2STFXfail.cmake | 6 ++ frontends/p4/simplifyDefUse.cpp | 16 ++++ midend/def_use.cpp | 69 ++++++++++---- midend/def_use.h | 26 ++++-- midend/unrollLoops.cpp | 84 ++++++++--------- midend/unrollLoops.h | 1 + testdata/p4_16_samples/issue4739.p4 | 91 +++++++++++++++++++ .../p4_16_samples_outputs/issue4739-first.p4 | 59 ++++++++++++ .../issue4739-frontend.p4 | 60 ++++++++++++ .../p4_16_samples_outputs/issue4739-midend.p4 | 87 ++++++++++++++++++ testdata/p4_16_samples_outputs/issue4739.p4 | 59 ++++++++++++ .../p4_16_samples_outputs/issue4739.p4-stderr | 0 .../issue4739.p4.entries.txtpb | 3 + .../issue4739.p4.p4info.txtpb | 6 ++ 17 files changed, 515 insertions(+), 70 deletions(-) create mode 100644 testdata/p4_16_samples/issue4739.p4 create mode 100644 testdata/p4_16_samples_outputs/issue4739-first.p4 create mode 100644 testdata/p4_16_samples_outputs/issue4739-frontend.p4 create mode 100644 testdata/p4_16_samples_outputs/issue4739-midend.p4 create mode 100644 testdata/p4_16_samples_outputs/issue4739.p4 create mode 100644 testdata/p4_16_samples_outputs/issue4739.p4-stderr create mode 100644 testdata/p4_16_samples_outputs/issue4739.p4.entries.txtpb create mode 100644 testdata/p4_16_samples_outputs/issue4739.p4.p4info.txtpb diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2MetadataXfail.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2MetadataXfail.cmake index 547b9acf58..7e02b790d3 100644 --- a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2MetadataXfail.cmake +++ b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2MetadataXfail.cmake @@ -57,6 +57,12 @@ p4tools_add_xfail_reason( # and more specifically when member is passed to getTypeArray ) +p4tools_add_xfail_reason( + "testgen-p4c-bmv2-metadata" + "Unhandled node type in Bmv2V1ModelCmdStepper: ForStatement" + issue4739.p4 +) + #################################################################################################### # 3. WONTFIX #################################################################################################### diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake index bf92ae661b..21ab40c8c2 100644 --- a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake +++ b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake @@ -101,6 +101,12 @@ p4tools_add_xfail_reason( issue907-bmv2.p4 ) +p4tools_add_xfail_reason( + "testgen-p4c-bmv2-ptf" + "Unhandled node type in Bmv2V1ModelCmdStepper: ForStatement" + issue4739.p4 +) + #################################################################################################### # 3. WONTFIX # These are failures that can not be solved by changing P4Testgen diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2ProtobufIrXfail.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2ProtobufIrXfail.cmake index 12706bdef8..9610640709 100644 --- a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2ProtobufIrXfail.cmake +++ b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2ProtobufIrXfail.cmake @@ -74,6 +74,12 @@ p4tools_add_xfail_reason( parser-unroll-test10.p4 ) +p4tools_add_xfail_reason( + "testgen-p4c-bmv2-protobuf-ir" + "Unhandled node type in Bmv2V1ModelCmdStepper: ForStatement" + issue4739.p4 +) + #################################################################################################### # 3. WONTFIX #################################################################################################### diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2STFXfail.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2STFXfail.cmake index 819ed8b0f3..3358286a43 100644 --- a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2STFXfail.cmake +++ b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2STFXfail.cmake @@ -101,6 +101,12 @@ p4tools_add_xfail_reason( issue907-bmv2.p4 ) +p4tools_add_xfail_reason( + "testgen-p4c-bmv2-stf" + "Unhandled node type in Bmv2V1ModelCmdStepper: ForStatement" + issue4739.p4 +) + #################################################################################################### # 3. WONTFIX # These are failures that can not be solved by changing P4Testgen diff --git a/frontends/p4/simplifyDefUse.cpp b/frontends/p4/simplifyDefUse.cpp index 579d206093..e13df64a54 100644 --- a/frontends/p4/simplifyDefUse.cpp +++ b/frontends/p4/simplifyDefUse.cpp @@ -948,6 +948,22 @@ class FindUninitialized : public Inspector { return setCurrent(statement); } + bool preorder(const IR::ForStatement *statement) override { + Log::TempIndent indent; + LOG3("FU Visiting " << dbp(statement) << " " << statement << indent); + if (!unreachable) { + visit(statement->init, "init"); + // use the live state from the end of the loop, as that jumps to the condition + setCurrent(statement); + visit(statement->condition, "condition"); + visit(statement->body, "body"); + visit(statement->updates, "updates"); + } else { + LOG3("Unreachable"); + } + return setCurrent(statement); + } + bool preorder(const IR::ForInStatement *statement) override { Log::TempIndent indent; LOG3("FU Visiting " << dbp(statement) << " " << statement << indent); diff --git a/midend/def_use.cpp b/midend/def_use.cpp index 9ce1728d1b..a616db9b89 100644 --- a/midend/def_use.cpp +++ b/midend/def_use.cpp @@ -18,19 +18,36 @@ limitations under the License. #include "frontends/p4/methodInstance.h" +struct LogAbbrev { + const Util::SourceInfo &si; + explicit LogAbbrev(const Util::SourceInfo &si) : si(si) {} + + friend std::ostream &operator<<(std::ostream &out, const LogAbbrev &la) { + if (la.si) { + unsigned line, col; + out << '(' << la.si.toSourcePositionData(&line, &col); + out << ':' << line << ':' << (col + 1) << ')'; + } + return out; + } +}; + namespace P4 { using namespace literals; +int ComputeDefUse::uid_ctr = 0; const ordered_set ComputeDefUse::empty = {}; void ComputeDefUse::flow_merge(Visitor &a_) { ComputeDefUse &a = dynamic_cast(a_); + LOG8("ComputeDefUse::flow_merge(" << a.uid << ") -> " << uid); unreachable &= a.unreachable; for (auto &di : a.def_info) def_info[di.first].flow_merge(di.second); } void ComputeDefUse::flow_copy(ControlFlowVisitor &a_) { ComputeDefUse &a = dynamic_cast(a_); + LOG8("ComputeDefUse::flow_copy(" << a.uid << ") -> " << uid); BUG_CHECK(state == a.state, "inconsistent state in ComputeDefUse::flow_copy"); unreachable = a.unreachable; def_info = a.def_info; @@ -264,7 +281,7 @@ void ComputeDefUse::def_info_t::split_slice(le_bitrange range) { bool ComputeDefUse::preorder(const IR::P4Control *c) { BUG_CHECK(state == SKIPPING, "Nested %s not supported in ComputeDefUse", c); IndentCtl::TempIndent indent; - LOG5("ComputeDefUse(P4Control " << c->name << ")" << indent); + LOG5("ComputeDefUse" << uid << "(P4Control " << c->name << ")" << indent); bool is_type_declaration = !c->getTypeParameters()->empty(); for (auto *p : c->getApplyParameters()->parameters) if (p->direction == IR::Direction::In || p->direction == IR::Direction::InOut) { @@ -293,7 +310,7 @@ bool ComputeDefUse::preorder(const IR::P4Control *c) { bool ComputeDefUse::preorder(const IR::P4Table *tbl) { if (state == SKIPPING) return false; IndentCtl::TempIndent indent; - LOG5("ComputeDefUse(P4Table " << tbl->name << ")" << indent); + LOG5("ComputeDefUse" << uid << "(P4Table " << tbl->name << ")" << indent); if (auto key = tbl->getKey()) visit(key, "key"); if (auto actions = tbl->getActionList()) { parallel_visit(actions->actionList, "actions"); @@ -307,7 +324,7 @@ bool ComputeDefUse::preorder(const IR::P4Action *act) { if (state == SKIPPING) return false; for (auto *p : *act->parameters) def_info[p].defs.insert(getLoc(p)); IndentCtl::TempIndent indent; - LOG5("ComputeDefUse(P4Action " << act->name << ")" << indent); + LOG5("ComputeDefUse" << uid << "(P4Action " << act->name << ")" << indent); visit(act->body, "body"); return false; } @@ -315,7 +332,7 @@ bool ComputeDefUse::preorder(const IR::P4Action *act) { bool ComputeDefUse::preorder(const IR::P4Parser *p) { BUG_CHECK(state == SKIPPING, "Nested %s not supported in ComputeDefUse", p); IndentCtl::TempIndent indent; - LOG5("ComputeDefUse(P4Parser " << p->name << ")" << indent); + LOG5("ComputeDefUse" << uid << "(P4Parser " << p->name << ")" << indent); for (auto *a : p->getApplyParameters()->parameters) if (a->direction == IR::Direction::In || a->direction == IR::Direction::InOut) def_info[a].defs.insert(getLoc(a)); @@ -333,7 +350,7 @@ bool ComputeDefUse::preorder(const IR::P4Parser *p) { return false; } bool ComputeDefUse::preorder(const IR::ParserState *p) { - LOG5("ComputeDefUse(ParserState " << p->name << ")" << Log::indent); + LOG5("ComputeDefUse" << uid << "(ParserState " << p->name << ")" << Log::indent); return true; } void ComputeDefUse::revisit(const IR::ParserState *p) { LOG5(" * revisit " << p->name); } @@ -345,6 +362,13 @@ bool ComputeDefUse::preorder(const IR::KeyElement *ke) { return false; } +bool ComputeDefUse::preorder(const IR::AssignmentStatement *as) { + // visit RHS of assignment before LHS + visit(as->right, "right", 1); + visit(as->left, "left", 0); + return false; +} + // Add all definitions in the given def_info_t (whole and partial) as defs that reach // a use at the specified location void ComputeDefUse::add_uses(const loc_t *loc, def_info_t &di) { @@ -396,6 +420,7 @@ static const IR::Expression *isValid(const IR::Member *m, const Visitor::Context const IR::Expression *ComputeDefUse::do_read(def_info_t &di, const IR::Expression *e, const Context *ctxt) { + LOG7("do_read(" << *e << "<" << e->id << ">" << LogAbbrev(e->srcInfo) << ")"); if (!ctxt) { } else if (auto *m = ctxt->node->to()) { if (auto *t = isValid(m, ctxt->parent)) { @@ -471,6 +496,7 @@ static int constIntMethodArg(const Visitor::Context *ctxt) { const IR::Expression *ComputeDefUse::do_write(def_info_t &di, const IR::Expression *e, const Context *ctxt) { + LOG7("do_write(" << *e << "<" << e->id << ">" << LogAbbrev(e->srcInfo) << ")"); if (!ctxt) { } else if (auto *m = ctxt->node->to()) { if (auto *t = isValid(m, ctxt->parent)) { @@ -566,7 +592,9 @@ const IR::Expression *ComputeDefUse::do_write(def_info_t &di, const IR::Expressi } bool ComputeDefUse::preorder(const IR::PathExpression *pe) { - LOG7("ComputeDefUse(PathExpression " << *pe << ")"); + Log::TempIndent indent; + LOG6("ComputeDefUse" << uid << "(PathExpression " << *pe << '<' << pe->id << '>' + << LogAbbrev(pe->srcInfo) << ")" << indent); if (pe->type->is()) { auto *d = resolveUnique(pe->path->name, P4::ResolutionType::Any); BUG_CHECK(d, "failed to resolve %s", pe); @@ -628,12 +656,18 @@ void ComputeDefUse::end_apply() { LOG5(defuse); } // Debugging std::ostream &operator<<(std::ostream &out, const ComputeDefUse::loc_t &loc) { - out << '<' << loc.node->id << '>'; - if (loc.node->srcInfo) { - unsigned line, col; - out << '(' << loc.node->srcInfo.toSourcePositionData(&line, &col); - out << ':' << line << ':' << (col + 1) << ')'; + out << '<' << loc.node->id << '>' << LogAbbrev(loc.node->srcInfo); + return out; +} + +std::ostream &operator<<(std::ostream &out, const ordered_set &s) { + out << ": {"; + const char *sep = " "; + for (auto *l : s) { + out << sep << *l; + sep = ", "; } + out << (sep + 1) << "}"; return out; } @@ -649,13 +683,7 @@ std::ostream &operator<<( out << '(' << p.first->srcInfo.toSourcePositionData(&line, &col); out << ':' << line << ':' << (col + 1) << ')'; } - out << ": {"; - const char *sep = " "; - for (auto *l : p.second) { - out << sep << *l; - sep = ", "; - } - out << (sep + 1) << "}"; + out << p.second; return out; } @@ -669,3 +697,8 @@ std::ostream &operator<<(std::ostream &out, const ComputeDefUse::defuse_t &du) { } } // namespace P4 + +void dump(const P4::ComputeDefUse::loc_t &p) { std::cout << p << std::endl; } +void dump(const ordered_set &p) { std::cout << p << std::endl; } +void dump(const P4::ComputeDefUse &du) { std::cout << du << std::endl; } +void dump(const P4::ComputeDefUse *du) { std::cout << *du << std::endl; } diff --git a/midend/def_use.h b/midend/def_use.h index e583b06e9c..6708a120dd 100644 --- a/midend/def_use.h +++ b/midend/def_use.h @@ -43,7 +43,14 @@ class ComputeDefUse : public Inspector, public ControlFlowVisitor, public P4WriteContext, public P4::ResolutionContext { - ComputeDefUse *clone() const override { return new ComputeDefUse(*this); } + static int uid_ctr; + int uid = 0; + ComputeDefUse *clone() const override { + auto *rv = new ComputeDefUse(*this); + rv->uid = ++uid_ctr; + LOG8("ComputeDefUse::clone " << rv->uid); + return rv; + } void flow_merge(Visitor &) override; void flow_copy(ControlFlowVisitor &) override; bool operator==(const ControlFlowVisitor &) const override; @@ -70,6 +77,7 @@ class ComputeDefUse : public Inspector, return nullptr; } }; + typedef ordered_set locset_t; private: std::set &cached_locs; @@ -83,13 +91,13 @@ class ComputeDefUse : public Inspector, // definitions of a symbol (or part of a symbol) visible at this point in the // program. `defs` will be empty if `live` is; if not those defs are visible only // for those bits/elements/fields where live is set. - ordered_set defs; + locset_t defs; bitvec live; // FIXME -- this parent field is never used and is not set consistently, so // appears to be useless? def_info_t *parent = nullptr; // track valid bit access for headers separate from the rest of the header - ordered_set valid_bit_defs; + locset_t valid_bit_defs; // one of these maps will always be empty. std::map fields; std::map slices; // also used for arrays @@ -114,14 +122,15 @@ class ComputeDefUse : public Inspector, // defs maps from all uses to their definitions // uses maps from all definitions to their uses // uses/defs are lvalue expressions, or param declarations. - ordered_map> defs; - ordered_map> uses; + ordered_map defs; + ordered_map uses; } & defuse; - static const ordered_set empty; + static const locset_t empty; profile_t init_apply(const IR::Node *root) override { auto rv = Inspector::init_apply(root); LOG3("## Midend ComputeDefUse"); + uid_ctr = 0; state = SKIPPING; clear(); return rv; @@ -137,6 +146,7 @@ class ComputeDefUse : public Inspector, bool preorder(const IR::Type *) override { return false; } bool preorder(const IR::Annotations *) override { return false; } bool preorder(const IR::KeyElement *) override; + bool preorder(const IR::AssignmentStatement *) override; const IR::Expression *do_read(def_info_t &, const IR::Expression *, const Context *); const IR::Expression *do_write(def_info_t &, const IR::Expression *, const Context *); bool preorder(const IR::PathExpression *) override; @@ -161,11 +171,11 @@ class ComputeDefUse : public Inspector, defuse.uses.clear(); } - const ordered_set &getDefs(const IR::Node *n) const { + const locset_t &getDefs(const IR::Node *n) const { auto it = defuse.defs.find(n); return it == defuse.defs.end() ? empty : it->second; } - const ordered_set &getUses(const IR::Node *n) const { + const locset_t &getUses(const IR::Node *n) const { auto it = defuse.uses.find(n); return it == defuse.uses.end() ? empty : it->second; } diff --git a/midend/unrollLoops.cpp b/midend/unrollLoops.cpp index 5dccee8c29..ba3e83a0df 100644 --- a/midend/unrollLoops.cpp +++ b/midend/unrollLoops.cpp @@ -122,47 +122,49 @@ class ReplaceIndexRefs : public Transform, P4WriteContext { ReplaceIndexRefs(cstring iv, long v) : indexVar(iv), value(v) { forceClone = true; } }; -static long evalLoop(const IR::Expression *exp, cstring index, long val, bool &fail) { +long UnrollLoops::evalLoop(const IR::Expression *exp, long val, + const ComputeDefUse::locset_t &idefs, bool &fail) { + if (fail) return 1; if (auto *pe = exp->to()) { - if (pe->path->name != index) fail = true; + if (defUse->getDefs(pe) != idefs) fail = true; return val; } else if (auto *k = exp->to()) { return k->asLong(); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) <= evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) <= evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) < evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) < evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) >= evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) >= evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) > evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) > evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) == evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) == evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) != evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) != evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) + evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) + evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) - evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) - evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) * evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) * evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) / evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) / evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) % evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) % evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) << evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) << evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return evalLoop(e->left, index, val, fail) >> evalLoop(e->right, index, val, fail); + return evalLoop(e->left, val, idefs, fail) >> evalLoop(e->right, val, idefs, fail); } else if (auto *e = exp->to()) { - return -evalLoop(e->expr, index, val, fail); + return -evalLoop(e->expr, val, idefs, fail); } else if (auto *e = exp->to()) { - return ~evalLoop(e->expr, index, val, fail); + return ~evalLoop(e->expr, val, idefs, fail); } else if (auto *e = exp->to()) { - return !evalLoop(e->expr, index, val, fail); + return !evalLoop(e->expr, val, idefs, fail); } fail = true; - return 0; + return 1; } bool UnrollLoops::findLoopBounds(IR::ForStatement *fstmt, loop_bounds_t &bounds) { @@ -172,33 +174,27 @@ bool UnrollLoops::findLoopBounds(IR::ForStatement *fstmt, loop_bounds_t &bounds) auto d = resolveUnique(v->path->name, P4::ResolutionType::Any); bounds.index = d ? d->to() : nullptr; if (!bounds.index) return false; - auto &defs = defUse->getDefs(v); const IR::AssignmentStatement *init = nullptr, *incr = nullptr; - for (auto *d : defs) { - bool ok = false; - for (auto *a : fstmt->init) { - if (a == d->parent->node) { - init = a->to(); - ok = true; - break; - } - } - for (auto *a : fstmt->updates) { - if (a == d->parent->node) { - incr = a->to(); - ok = true; - break; - } - } - if (!ok) return false; + const IR::Constant *initval = nullptr; + auto &index_defs = defUse->getDefs(v); + if (index_defs.size() != 2) { + // must be exactly 2 defs -- one to a constant before the loop and one + // to a non-constant in the loop + return false; + } else if ((init = index_defs.front()->parent->node->to()) && + (initval = init->right->to())) { + incr = index_defs.back()->parent->node->to(); + } else if ((init = index_defs.back()->parent->node->to()) && + (initval = init->right->to())) { + incr = index_defs.front()->parent->node->to(); + } else { + return false; } - if (!init || !incr) return false; - auto initval = init->right->to(); - if (!initval) return false; + if (!incr) return false; bool fail = false; for (long val = initval->asLong(); - evalLoop(fstmt->condition, bounds.index->name, val, fail) && !fail; - val = evalLoop(incr->right, bounds.index->name, val, fail)) { + evalLoop(fstmt->condition, val, index_defs, fail) && !fail; + val = evalLoop(incr->right, val, index_defs, fail)) { if (bounds.indexes.size() > 1000) { fail = true; break; @@ -262,7 +258,7 @@ const IR::Statement *UnrollLoops::doUnroll(const loop_bounds_t &bounds, const IR const IR::Statement *UnrollLoops::preorder(IR::ForStatement *fstmt) { loop_bounds_t bounds; if (findLoopBounds(fstmt, bounds)) { - LOG4("Unrolling loop" << Log::indent << Log::endl << fstmt << Log::unindent); + LOG3("Unrolling loop" << Log::indent << Log::endl << fstmt << Log::unindent); auto *rv = new IR::BlockStatement; for (auto *i : fstmt->init) rv->append(i); rv->append(doUnroll(bounds, fstmt->body, &fstmt->updates)); @@ -275,7 +271,7 @@ const IR::Statement *UnrollLoops::preorder(IR::ForStatement *fstmt) { const IR::Statement *UnrollLoops::preorder(IR::ForInStatement *fstmt) { loop_bounds_t bounds; if (findLoopBounds(fstmt, bounds)) { - LOG4("Unrolling loop" << Log::indent << Log::endl << fstmt << Log::unindent); + LOG3("Unrolling loop" << Log::indent << Log::endl << fstmt << Log::unindent); auto rv = doUnroll(bounds, fstmt->body); LOG4("Unrolled loop" << Log::indent << Log::endl << rv << Log::unindent); return rv; diff --git a/midend/unrollLoops.h b/midend/unrollLoops.h index 40d9fb258f..9d9fec4180 100644 --- a/midend/unrollLoops.h +++ b/midend/unrollLoops.h @@ -30,6 +30,7 @@ class UnrollLoops : public Transform, public P4::ResolutionContext { const IR::Declaration_Variable *index = nullptr; std::vector indexes; }; + long evalLoop(const IR::Expression *, long, const ComputeDefUse::locset_t &, bool &); bool findLoopBounds(IR::ForStatement *, loop_bounds_t &); bool findLoopBounds(IR::ForInStatement *, loop_bounds_t &); const IR::Statement *doUnroll(const loop_bounds_t &, const IR::Statement *, diff --git a/testdata/p4_16_samples/issue4739.p4 b/testdata/p4_16_samples/issue4739.p4 new file mode 100644 index 0000000000..3f8a066477 --- /dev/null +++ b/testdata/p4_16_samples/issue4739.p4 @@ -0,0 +1,91 @@ +/* +Copyright 2024 Andy Fingerhut + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include +#include + +header ethernet_t { + bit<48> dstAddr; + bit<48> srcAddr; + bit<16> etherType; +} + +struct metadata_t { +} + +struct headers_t { + ethernet_t ethernet; +} + +parser parserImpl(packet_in packet, + out headers_t hdr, + inout metadata_t meta, + inout standard_metadata_t stdmeta) +{ + state start { + packet.extract(hdr.ethernet); + transition accept; + } +} + +control ingressImpl(inout headers_t hdr, + inout metadata_t meta, + inout standard_metadata_t stdmeta) +{ + bit<8> n; + apply { + n = 0; + for (bit<8> i = 0; i < 8; i = i + 1) { + i = i + 1; + n = n + 1; + } + hdr.ethernet.srcAddr[7:0] = n; + stdmeta.egress_spec = 1; + } +} + +control egressImpl(inout headers_t hdr, + inout metadata_t meta, + inout standard_metadata_t stdmeta) +{ + apply { + } +} + +control deparserImpl(packet_out packet, + in headers_t hdr) +{ + apply { + packet.emit(hdr.ethernet); + } +} + +control verifyChecksum(inout headers_t hdr, inout metadata_t meta) { + apply { + } +} + +control updateChecksum(inout headers_t hdr, inout metadata_t meta) { + apply { + } +} + +V1Switch(parserImpl(), + verifyChecksum(), + ingressImpl(), + egressImpl(), + updateChecksum(), + deparserImpl()) main; diff --git a/testdata/p4_16_samples_outputs/issue4739-first.p4 b/testdata/p4_16_samples_outputs/issue4739-first.p4 new file mode 100644 index 0000000000..62d54f6bd8 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue4739-first.p4 @@ -0,0 +1,59 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header ethernet_t { + bit<48> dstAddr; + bit<48> srcAddr; + bit<16> etherType; +} + +struct metadata_t { +} + +struct headers_t { + ethernet_t ethernet; +} + +parser parserImpl(packet_in packet, out headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + state start { + packet.extract(hdr.ethernet); + transition accept; + } +} + +control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + bit<8> n; + apply { + n = 8w0; + for (bit<8> i = 8w0; i < 8w8; i = i + 8w1) { + i = i + 8w1; + n = n + 8w1; + } + hdr.ethernet.srcAddr[7:0] = n; + stdmeta.egress_spec = 9w1; + } +} + +control egressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control deparserImpl(packet_out packet, in headers_t hdr) { + apply { + packet.emit(hdr.ethernet); + } +} + +control verifyChecksum(inout headers_t hdr, inout metadata_t meta) { + apply { + } +} + +control updateChecksum(inout headers_t hdr, inout metadata_t meta) { + apply { + } +} + +V1Switch(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main; diff --git a/testdata/p4_16_samples_outputs/issue4739-frontend.p4 b/testdata/p4_16_samples_outputs/issue4739-frontend.p4 new file mode 100644 index 0000000000..d845b7b1e8 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue4739-frontend.p4 @@ -0,0 +1,60 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header ethernet_t { + bit<48> dstAddr; + bit<48> srcAddr; + bit<16> etherType; +} + +struct metadata_t { +} + +struct headers_t { + ethernet_t ethernet; +} + +parser parserImpl(packet_in packet, out headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + state start { + packet.extract(hdr.ethernet); + transition accept; + } +} + +control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + @name("ingressImpl.n") bit<8> n_0; + @name("ingressImpl.i") bit<8> i_0; + apply { + n_0 = 8w0; + for (i_0 = 8w0; i_0 < 8w8; i_0 = i_0 + 8w1) { + i_0 = i_0 + 8w1; + n_0 = n_0 + 8w1; + } + hdr.ethernet.srcAddr[7:0] = n_0; + stdmeta.egress_spec = 9w1; + } +} + +control egressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control deparserImpl(packet_out packet, in headers_t hdr) { + apply { + packet.emit(hdr.ethernet); + } +} + +control verifyChecksum(inout headers_t hdr, inout metadata_t meta) { + apply { + } +} + +control updateChecksum(inout headers_t hdr, inout metadata_t meta) { + apply { + } +} + +V1Switch(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main; diff --git a/testdata/p4_16_samples_outputs/issue4739-midend.p4 b/testdata/p4_16_samples_outputs/issue4739-midend.p4 new file mode 100644 index 0000000000..ed6c874ed3 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue4739-midend.p4 @@ -0,0 +1,87 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header ethernet_t { + bit<48> dstAddr; + bit<48> srcAddr; + bit<16> etherType; +} + +struct metadata_t { +} + +struct headers_t { + ethernet_t ethernet; +} + +parser parserImpl(packet_in packet, out headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + state start { + packet.extract(hdr.ethernet); + transition accept; + } +} + +control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + @name("ingressImpl.n") bit<8> n_0; + @name("ingressImpl.i") bit<8> i_0; + @hidden action issue4739l52() { + i_0 = i_0 + 8w1; + n_0 = n_0 + 8w1; + } + @hidden action issue4739l50() { + n_0 = 8w0; + } + @hidden action issue4739l55() { + hdr.ethernet.srcAddr[7:0] = n_0; + stdmeta.egress_spec = 9w1; + } + @hidden table tbl_issue4739l50 { + actions = { + issue4739l50(); + } + const default_action = issue4739l50(); + } + @hidden table tbl_issue4739l52 { + actions = { + issue4739l52(); + } + const default_action = issue4739l52(); + } + @hidden table tbl_issue4739l55 { + actions = { + issue4739l55(); + } + const default_action = issue4739l55(); + } + apply { + tbl_issue4739l50.apply(); + for (i_0 = 8w0; i_0 < 8w8; i_0 = i_0 + 8w1) { + tbl_issue4739l52.apply(); + } + tbl_issue4739l55.apply(); + } +} + +control egressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control deparserImpl(packet_out packet, in headers_t hdr) { + apply { + packet.emit(hdr.ethernet); + } +} + +control verifyChecksum(inout headers_t hdr, inout metadata_t meta) { + apply { + } +} + +control updateChecksum(inout headers_t hdr, inout metadata_t meta) { + apply { + } +} + +V1Switch(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main; diff --git a/testdata/p4_16_samples_outputs/issue4739.p4 b/testdata/p4_16_samples_outputs/issue4739.p4 new file mode 100644 index 0000000000..05161d7466 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue4739.p4 @@ -0,0 +1,59 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header ethernet_t { + bit<48> dstAddr; + bit<48> srcAddr; + bit<16> etherType; +} + +struct metadata_t { +} + +struct headers_t { + ethernet_t ethernet; +} + +parser parserImpl(packet_in packet, out headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + state start { + packet.extract(hdr.ethernet); + transition accept; + } +} + +control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + bit<8> n; + apply { + n = 0; + for (bit<8> i = 0; i < 8; i = i + 1) { + i = i + 1; + n = n + 1; + } + hdr.ethernet.srcAddr[7:0] = n; + stdmeta.egress_spec = 1; + } +} + +control egressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + apply { + } +} + +control deparserImpl(packet_out packet, in headers_t hdr) { + apply { + packet.emit(hdr.ethernet); + } +} + +control verifyChecksum(inout headers_t hdr, inout metadata_t meta) { + apply { + } +} + +control updateChecksum(inout headers_t hdr, inout metadata_t meta) { + apply { + } +} + +V1Switch(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main; diff --git a/testdata/p4_16_samples_outputs/issue4739.p4-stderr b/testdata/p4_16_samples_outputs/issue4739.p4-stderr new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testdata/p4_16_samples_outputs/issue4739.p4.entries.txtpb b/testdata/p4_16_samples_outputs/issue4739.p4.entries.txtpb new file mode 100644 index 0000000000..5cb9652623 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue4739.p4.entries.txtpb @@ -0,0 +1,3 @@ +# proto-file: p4/v1/p4runtime.proto +# proto-message: p4.v1.WriteRequest + diff --git a/testdata/p4_16_samples_outputs/issue4739.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/issue4739.p4.p4info.txtpb new file mode 100644 index 0000000000..fdf16790b9 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue4739.p4.p4info.txtpb @@ -0,0 +1,6 @@ +# proto-file: p4/config/v1/p4info.proto +# proto-message: p4.config.v1.P4Info + +pkg_info { + arch: "v1model" +}