From bdbe514de9906a3b12c1972ba903f2562773edeb Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Mon, 30 Dec 2019 18:46:51 -0800 Subject: [PATCH] Support lvalue expressions The bmv2 expression engine can now returns references to bmv2 primitive objects (Data, Header, ...) when the expression being evaluated is an lvalue. This means that we can now handle assignments to complex lvalue expressions which do not statically resolve to a field / header / union. One example would be an assignment to a header stack field accessed through a runtime index. Unit tests were added, although at the moment they do not support all the possible cases (e.g. expressions which evaluate to a header union). The JSON version number was bumped up to 2.23. Even though no additions were made to the format, we now support additional cases and the documentation now includes an example of a "complex" assignment statement. Fixes #838 --- docs/JSON_format.md | 61 +++- include/bm/bm_sim/P4Objects.h | 2 - include/bm/bm_sim/actions.h | 86 +++-- include/bm/bm_sim/expressions.h | 43 ++- src/bm_sim/P4Objects.cpp | 130 +------ src/bm_sim/actions.cpp | 46 ++- src/bm_sim/expressions.cpp | 592 +++++++++++++++++++++----------- tests/test_actions.cpp | 46 ++- tests/test_conditionals.cpp | 7 +- tests/test_expressions.cpp | 47 ++- 10 files changed, 695 insertions(+), 365 deletions(-) diff --git a/docs/JSON_format.md b/docs/JSON_format.md index 16e5b89d6..9ce673d6d 100644 --- a/docs/JSON_format.md +++ b/docs/JSON_format.md @@ -10,7 +10,7 @@ on each attribute. ## Current bmv2 JSON format version -The version described in this document is *2.22*. +The version described in this document is *2.23*. The major version number will be increased by the compiler only when backward-compatibility of the JSON format is broken. After a major version @@ -464,6 +464,65 @@ call, with the following attributes: of action) array. If `type` is `extern`, this is the name of the extern instance. See [here](#the-type-value-object) for other types. +An `expression` can either correspond to an lvalue expression or an rvalue +expression. For example, the P4 expression `hdr.h2[hdr.h1.idx].v = hdr.h1.v + 7` +corresponds to the following JSON object (which would be an entry in the +`primitives` JSON array for the action to which the expression belongs): +```json +{ + "op" : "assign", + "parameters" : [ + { + "type" : "expression", + "value": { + "type": "expression", + "value": { + "op": "access_field", + "left": { + "type": "expression", + "value": { + "op": "dereference_header_stack", + "left": { + "type": "header_stack", + "value": "h2" + }, + "right": { + "type": "field", + "value": ["h1", "idx"] + } + } + }, + "right": 0 + } + } + }, + { + "type" : "expression", + "value": { + "type": "expression", + "value": { + "op": "+", + "left": { + "type" : "field", + "value" : ["h1", "v"] + }, + "right": { + "type" : "hexstr", + "value" : "0x0000004d" + } + } + } + } + ] +} +``` +In this case, the left-hand side of the assignment (`hdr.h2[hdr.h1.idx].v`) is +an lvalue, while the right-hand side is an rvalue (`hdr.h1.v + 77`). However, +this information does not need to appear *explicitly* in the JSON. The bmv2 +implementation of the `assign` primitive requires the first parameter to be a +lvalue expression, and we assume that the compiler will not generate a JSON that +violates this requirement. An invalid JSON will lead to undefined behavior. + *Important note about extern instance methods*: even though in P4 these are invoked using object-oriented style, bmv2 treats them as regular primitives for which the first parameter is the extern instance. For example, if the P4 call is diff --git a/include/bm/bm_sim/P4Objects.h b/include/bm/bm_sim/P4Objects.h index f051e99ed..f1e6cb337 100644 --- a/include/bm/bm_sim/P4Objects.h +++ b/include/bm/bm_sim/P4Objects.h @@ -92,8 +92,6 @@ class P4Objects { std::set headers; }; - enum class ExprType; // forward declaration - public: // NOLINTNEXTLINE(runtime/references) explicit P4Objects(std::ostream &outstream, bool verbose_output = false); diff --git a/include/bm/bm_sim/actions.h b/include/bm/bm_sim/actions.h index 45a74bca3..8a11f61a0 100644 --- a/include/bm/bm_sim/actions.h +++ b/include/bm/bm_sim/actions.h @@ -1,4 +1,5 @@ -/* Copyright 2013-present Barefoot Networks, Inc. +/* Copyright 2013-2019 Barefoot Networks, Inc. + * Copyright 2019 VMware, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +15,7 @@ */ /* - * Antonin Bas (antonin@barefootnetworks.com) + * Antonin Bas * */ @@ -175,7 +176,8 @@ struct ActionParam { HEADER_STACK, LAST_HEADER_STACK_FIELD, CALCULATION, METER_ARRAY, COUNTER_ARRAY, REGISTER_ARRAY, - EXPRESSION, + EXPRESSION, EXPRESSION_HEADER, EXPRESSION_HEADER_STACK, + EXPRESSION_HEADER_UNION, EXPRESSION_HEADER_UNION_STACK, EXTERN_INSTANCE, STRING, HEADER_UNION, HEADER_UNION_STACK, PARAMS_VECTOR} tag; @@ -234,17 +236,17 @@ struct ActionParam { RegisterArray *register_array; struct { - unsigned int offset; + unsigned int offset; // only used for arithmetic ("Data") expressions // non owning pointer // in theory, could be an owning pointer, but the union makes this // complicated, so instead the ActionFn keeps a vector of owning pointers - ArithExpression *ptr; + Expression *ptr; } expression; ExternType *extern_instance; // I use a pointer here to avoid complications with the union; the string - // memory is owned by ActionFn (just like for ArithExpression above) + // memory is owned by ActionFn (just like for Expression above) const std::string *str; header_union_stack_id_t header_union_stack; @@ -273,7 +275,6 @@ struct ActionEngineState { // they have to be declared outside of the class declaration, and "inline" is // necessary to avoid linker errors -// can only be a field or a register reference template <> inline Data &ActionParam::to(ActionEngineState *state) const { static thread_local Data data_temp; @@ -284,9 +285,12 @@ Data &ActionParam::to(ActionEngineState *state) const { case ActionParam::REGISTER_REF: return register_ref.array->at(register_ref.idx); case ActionParam::REGISTER_GEN: - register_gen.idx->eval(state->phv, &data_temp, - state->action_data.action_data); + register_gen.idx->eval_arith(state->phv, &data_temp, + state->action_data.action_data); return register_ref.array->at(data_temp.get()); + case ActionParam::EXPRESSION: + return expression.ptr->eval_arith_lvalue(&state->phv, + state->action_data.action_data); case ActionParam::LAST_HEADER_STACK_FIELD: return state->phv.get_header_stack(stack_field.header_stack).get_last() .get_field(stack_field.field_offset); @@ -312,16 +316,16 @@ const Data &ActionParam::to(ActionEngineState *state) const { case ActionParam::REGISTER_REF: return register_ref.array->at(register_ref.idx); case ActionParam::REGISTER_GEN: - register_gen.idx->eval(state->phv, &data_temps[0], - state->action_data.action_data); + register_gen.idx->eval_arith(state->phv, &data_temps[0], + state->action_data.action_data); return register_ref.array->at(data_temps[0].get()); case ActionParam::EXPRESSION: while (data_temps_size <= expression.offset) { data_temps.emplace_back(); data_temps_size++; } - expression.ptr->eval(state->phv, &data_temps[expression.offset], - state->action_data.action_data); + expression.ptr->eval_arith(state->phv, &data_temps[expression.offset], + state->action_data.action_data); return data_temps[expression.offset]; case ActionParam::LAST_HEADER_STACK_FIELD: return state->phv.get_header_stack(stack_field.header_stack).get_last() @@ -347,8 +351,15 @@ const Field &ActionParam::to(ActionEngineState *state) const { template <> inline Header &ActionParam::to
(ActionEngineState *state) const { - assert(tag == ActionParam::HEADER); - return state->phv.get_header(header); + switch (tag) { + case ActionParam::HEADER: + return state->phv.get_header(header); + case ActionParam::EXPRESSION_HEADER: + return expression.ptr->eval_header(&state->phv, + state->action_data.action_data); + default: + _BM_UNREACHABLE("Default switch case should not be reachable"); + } } template <> inline @@ -358,8 +369,15 @@ const Header &ActionParam::to(ActionEngineState *state) const { template <> inline HeaderStack &ActionParam::to(ActionEngineState *state) const { - assert(tag == ActionParam::HEADER_STACK); - return state->phv.get_header_stack(header_stack); + switch (tag) { + case ActionParam::HEADER_STACK: + return state->phv.get_header_stack(header_stack); + case ActionParam::EXPRESSION_HEADER_STACK: + return expression.ptr->eval_header_stack(&state->phv, + state->action_data.action_data); + default: + _BM_UNREACHABLE("Default switch case should not be reachable"); + } } template <> inline @@ -373,8 +391,14 @@ StackIface &ActionParam::to(ActionEngineState *state) const { switch (tag) { case HEADER_STACK: return state->phv.get_header_stack(header_stack); + case ActionParam::EXPRESSION_HEADER_STACK: + return expression.ptr->eval_header_stack( + &state->phv, state->action_data.action_data); case HEADER_UNION_STACK: return state->phv.get_header_union_stack(header_union_stack); + case ActionParam::EXPRESSION_HEADER_UNION_STACK: + return expression.ptr->eval_header_union_stack( + &state->phv, state->action_data.action_data); default: _BM_UNREACHABLE("Default switch case should not be reachable"); } @@ -388,8 +412,15 @@ const StackIface &ActionParam::to( template <> inline HeaderUnion &ActionParam::to(ActionEngineState *state) const { - assert(tag == ActionParam::HEADER_UNION); - return state->phv.get_header_union(header_union); + switch (tag) { + case ActionParam::HEADER_UNION: + return state->phv.get_header_union(header_union); + case ActionParam::EXPRESSION_HEADER_UNION: + return expression.ptr->eval_header_union(&state->phv, + state->action_data.action_data); + default: + _BM_UNREACHABLE("Default switch case should not be reachable"); + } } template <> inline @@ -401,8 +432,15 @@ const HeaderUnion &ActionParam::to( template <> inline HeaderUnionStack &ActionParam::to( ActionEngineState *state) const { - assert(tag == ActionParam::HEADER_UNION_STACK); - return state->phv.get_header_union_stack(header_union_stack); + switch (tag) { + case ActionParam::HEADER_UNION_STACK: + return state->phv.get_header_union_stack(header_union_stack); + case ActionParam::EXPRESSION_HEADER_UNION_STACK: + return expression.ptr->eval_header_union_stack( + &state->phv, state->action_data.action_data); + default: + _BM_UNREACHABLE("Default switch case should not be reachable"); + } } template <> inline @@ -698,7 +736,9 @@ class ActionFn : public NamedP4Object { void parameter_push_back_meter_array(MeterArray *meter_array); void parameter_push_back_counter_array(CounterArray *counter_array); void parameter_push_back_register_array(RegisterArray *register_array); - void parameter_push_back_expression(std::unique_ptr expr); + void parameter_push_back_expression(std::unique_ptr expr); + void parameter_push_back_expression(std::unique_ptr expr, + ExprType expr_type); void parameter_push_back_extern_instance(ExternType *extern_instance); void parameter_push_back_string(const std::string &str); @@ -727,7 +767,7 @@ class ActionFn : public NamedP4Object { RegisterSync register_sync{}; std::vector const_values{}; // should I store the objects in the vector, instead of pointers? - std::vector > expressions{}; + std::vector > expressions{}; std::vector strings{}; size_t num_params; diff --git a/include/bm/bm_sim/expressions.h b/include/bm/bm_sim/expressions.h index 1657ab50d..91aea35a4 100644 --- a/include/bm/bm_sim/expressions.h +++ b/include/bm/bm_sim/expressions.h @@ -1,4 +1,5 @@ -/* Copyright 2013-present Barefoot Networks, Inc. +/* Copyright 2013-2019 Barefoot Networks, Inc. + * Copyright 2019 VMware, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +15,7 @@ */ /* - * Antonin Bas (antonin@barefootnetworks.com) + * Antonin Bas * */ @@ -26,13 +27,18 @@ #include #include "data.h" +#include "headers.h" +#include "header_unions.h" #include "phv_forward.h" +#include "stacks.h" namespace bm { class RegisterArray; class RegisterSync; +struct ExpressionTemps; + enum class ExprOpcode { LOAD_FIELD, LOAD_HEADER, LOAD_HEADER_STACK, LOAD_LAST_HEADER_STACK_FIELD, LOAD_UNION, LOAD_UNION_STACK, LOAD_BOOL, LOAD_CONST, LOAD_LOCAL, @@ -56,6 +62,10 @@ enum class ExprOpcode { ACCESS_UNION_HEADER, }; +enum class ExprType { + UNKNOWN, DATA, HEADER, HEADER_STACK, BOOL, UNION, UNION_STACK +}; + class ExprOpcodesMap { public: static ExprOpcode get_opcode(std::string expr_name); @@ -69,6 +79,14 @@ class ExprOpcodesMap { std::unordered_map opcodes_map{}; }; +class ExprOpcodesUtils { + public: + static ExprOpcode get_eq_opcode(ExprType expr_type); + static ExprOpcode get_neq_opcode(ExprType expr_type); + + static ExprType get_opcode_type(ExprOpcode opcode); +}; + struct Op { ExprOpcode opcode; @@ -122,6 +140,8 @@ class Expression { public: Expression(); + virtual ~Expression() { } + void push_back_load_field(header_id_t header, int field_offset); void push_back_load_bool(bool value); void push_back_load_header(header_id_t header); @@ -149,6 +169,14 @@ class Expression { Data eval_arith(const PHV &phv, const std::vector &locals = {}) const; void eval_arith(const PHV &phv, Data *data, const std::vector &locals = {}) const; + Data &eval_arith_lvalue(PHV *phv, const std::vector &locals = {}) const; + Header &eval_header(PHV *phv, const std::vector &locals = {}) const; + HeaderStack &eval_header_stack( + PHV *phv, const std::vector &locals = {}) const; + HeaderUnion &eval_header_union( + PHV *phv, const std::vector &locals = {}) const; + HeaderUnionStack &eval_header_union_stack( + PHV *phv, const std::vector &locals = {}) const; bool empty() const; @@ -159,14 +187,11 @@ class Expression { Expression(Expression &&other) /*noexcept*/ = default; Expression &operator=(Expression &&other) /*noexcept*/ = default; - private: - enum class ExprType {EXPR_BOOL, EXPR_DATA}; - private: int assign_dest_registers(); - void eval_(const PHV &phv, ExprType expr_type, + void eval_(const PHV &phv, const std::vector &locals, - bool *b_res, Data *d_res) const; + ExpressionTemps *temps) const; size_t get_num_ops() const; void append_expression(const Expression &e); @@ -186,6 +211,10 @@ class ArithExpression : public Expression { const std::vector &locals = {}) const { eval_arith(phv, data, locals); } + + Data &eval_lvalue(PHV *phv, const std::vector &locals = {}) const { + return eval_arith_lvalue(phv, locals); + } }; diff --git a/src/bm_sim/P4Objects.cpp b/src/bm_sim/P4Objects.cpp index 24d7f03fb..ccd2fb658 100644 --- a/src/bm_sim/P4Objects.cpp +++ b/src/bm_sim/P4Objects.cpp @@ -1,4 +1,5 @@ -/* Copyright 2013-present Barefoot Networks, Inc. +/* Copyright 2013-2019 Barefoot Networks, Inc. + * Copyright 2019 VMware, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +15,7 @@ */ /* - * Antonin Bas (antonin@barefootnetworks.com) + * Antonin Bas * */ @@ -43,7 +44,7 @@ using std::string; namespace { constexpr int required_major_version = 2; -constexpr int max_minor_version = 21; +constexpr int max_minor_version = 23; // not needed for now // constexpr int min_minor_version = 0; @@ -117,10 +118,6 @@ using EFormat = ExceptionFormatter; } // namespace -enum class P4Objects::ExprType { - UNKNOWN, DATA, HEADER, HEADER_STACK, BOOL, UNION, UNION_STACK -}; - std::string P4Objects::get_json_version_string() { return get_version_str(required_major_version, max_minor_version); @@ -129,116 +126,12 @@ P4Objects::get_json_version_string() { void P4Objects::build_expression(const Json::Value &json_expression, Expression *expr) { - ExprType expr_type(ExprType::UNKNOWN); + ExprType expr_type; build_expression(json_expression, expr, &expr_type); } namespace { -using ExprType = P4Objects::ExprType; - -ExprOpcode get_eq_opcode(ExprType expr_type) { - switch (expr_type) { - case ExprType::DATA: - return ExprOpcode::EQ_DATA; - case ExprType::HEADER: - return ExprOpcode::EQ_HEADER; - case ExprType::BOOL: - return ExprOpcode::EQ_BOOL; - case ExprType::UNION: - return ExprOpcode::EQ_UNION; - default: - break; - } - assert(0); - return ExprOpcode::EQ_DATA; -} - -ExprOpcode get_neq_opcode(ExprType expr_type) { - switch (expr_type) { - case ExprType::DATA: - return ExprOpcode::NEQ_DATA; - case ExprType::HEADER: - return ExprOpcode::NEQ_HEADER; - case ExprType::BOOL: - return ExprOpcode::NEQ_BOOL; - case ExprType::UNION: - return ExprOpcode::NEQ_UNION; - default: - break; - } - assert(0); - return ExprOpcode::NEQ_DATA; -} - -// TODO(antonin): should this information be in expressions.h? -ExprType get_opcode_type(ExprOpcode opcode) { - switch (opcode) { - case ExprOpcode::LOAD_FIELD: - case ExprOpcode::LOAD_CONST: - case ExprOpcode::LOAD_LOCAL: - case ExprOpcode::LOAD_REGISTER_REF: - case ExprOpcode::LOAD_REGISTER_GEN: - case ExprOpcode::LOAD_LAST_HEADER_STACK_FIELD: - case ExprOpcode::ADD: - case ExprOpcode::SUB: - case ExprOpcode::MOD: - case ExprOpcode::DIV: - case ExprOpcode::MUL: - case ExprOpcode::SHIFT_LEFT: - case ExprOpcode::SHIFT_RIGHT: - case ExprOpcode::BIT_AND: - case ExprOpcode::BIT_OR: - case ExprOpcode::BIT_XOR: - case ExprOpcode::BIT_NEG: - case ExprOpcode::TWO_COMP_MOD: - case ExprOpcode::USAT_CAST: - case ExprOpcode::SAT_CAST: - case ExprOpcode::BOOL_TO_DATA: - case ExprOpcode::LAST_STACK_INDEX: - case ExprOpcode::SIZE_STACK: - case ExprOpcode::ACCESS_FIELD: - return ExprType::DATA; - case ExprOpcode::LOAD_BOOL: - case ExprOpcode::EQ_DATA: - case ExprOpcode::NEQ_DATA: - case ExprOpcode::GT_DATA: - case ExprOpcode::LT_DATA: - case ExprOpcode::GET_DATA: - case ExprOpcode::LET_DATA: - case ExprOpcode::EQ_HEADER: - case ExprOpcode::NEQ_HEADER: - case ExprOpcode::EQ_UNION: - case ExprOpcode::NEQ_UNION: - case ExprOpcode::EQ_BOOL: - case ExprOpcode::NEQ_BOOL: - case ExprOpcode::AND: - case ExprOpcode::OR: - case ExprOpcode::NOT: - case ExprOpcode::VALID_HEADER: - case ExprOpcode::VALID_UNION: - case ExprOpcode::DATA_TO_BOOL: - return ExprType::BOOL; - case ExprOpcode::LOAD_HEADER: - case ExprOpcode::DEREFERENCE_HEADER_STACK: - case ExprOpcode::ACCESS_UNION_HEADER: - return ExprType::HEADER; - case ExprOpcode::LOAD_HEADER_STACK: - return ExprType::HEADER_STACK; - case ExprOpcode::LOAD_UNION: - case ExprOpcode::DEREFERENCE_UNION_STACK: - return ExprType::UNION; - case ExprOpcode::LOAD_UNION_STACK: - return ExprType::UNION_STACK; - case ExprOpcode::TERNARY_OP: - return ExprType::UNKNOWN; - case ExprOpcode::SKIP: - break; - } - assert(0); - return ExprType::UNKNOWN; -} - std::unique_ptr object_source_info(const Json::Value &cfg_object) { std::string filename = ""; unsigned int line = 0; @@ -297,7 +190,9 @@ P4Objects::build_expression(const Json::Value &json_expression, build_expression(json_right, expr, &typeR); assert(typeL == typeR); assert(typeL != ExprType::UNKNOWN); - auto opcode = (op == "==") ? get_eq_opcode(typeL) : get_neq_opcode(typeL); + auto opcode = (op == "==") ? + ExprOpcodesUtils::get_eq_opcode(typeL) : + ExprOpcodesUtils::get_neq_opcode(typeL); expr->push_back_op(opcode); *expr_type = ExprType::BOOL; } else if (op == "access_field") { @@ -313,7 +208,7 @@ P4Objects::build_expression(const Json::Value &json_expression, auto opcode = ExprOpcodesMap::get_opcode(op); expr->push_back_op(opcode); - *expr_type = get_opcode_type(opcode); + *expr_type = ExprOpcodesUtils::get_opcode_type(opcode); } } else if (type == "header") { auto header_id = get_header_id_cfg(json_value.asString()); @@ -491,11 +386,12 @@ P4Objects::process_single_param(ActionFn* action_fn, } else if (type == "expression") { // TODO(Antonin): should this make the field case (and other) obsolete // maybe if we can optimize this case - auto expr = new ArithExpression(); - build_expression(cfg_parameter["value"], expr); + auto expr = new Expression(); + ExprType expr_type; + build_expression(cfg_parameter["value"], expr, &expr_type); expr->build(); action_fn->parameter_push_back_expression( - std::unique_ptr(expr)); + std::unique_ptr(expr)); } else if (type == "register") { // TODO(antonin): cheap optimization // this may not be worth doing, and probably does not belong here diff --git a/src/bm_sim/actions.cpp b/src/bm_sim/actions.cpp index b4c053730..3efdaa4e2 100644 --- a/src/bm_sim/actions.cpp +++ b/src/bm_sim/actions.cpp @@ -1,4 +1,5 @@ -/* Copyright 2013-present Barefoot Networks, Inc. +/* Copyright 2013-2019 Barefoot Networks, Inc. + * Copyright 2019 VMware, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +15,7 @@ */ /* - * Antonin Bas (antonin@barefootnetworks.com) + * Antonin Bas * */ @@ -140,10 +141,12 @@ ActionFn::parameter_push_back_register_gen( idx->grab_register_accesses(®ister_sync); - expressions.push_back(std::move(idx)); - param.register_gen.idx = expressions.back().get(); + param.register_gen.idx = idx.get(); params.push_back(param); + // does not invalidate param.register_gen.idx + expressions.push_back(std::move(idx)); + register_sync.add_register_array(register_array); } @@ -182,9 +185,7 @@ ActionFn::parameter_push_back_register_array(RegisterArray *register_array) { } void -ActionFn::parameter_push_back_expression( - std::unique_ptr expr -) { +ActionFn::parameter_push_back_expression(std::unique_ptr expr) { size_t nb_expression_params = 1; for (const auto &p : params) if (p.tag == ActionParam::EXPRESSION) nb_expression_params += 1; @@ -203,6 +204,37 @@ ActionFn::parameter_push_back_expression( params.push_back(param); } +void +ActionFn::parameter_push_back_expression(std::unique_ptr expr, + ExprType expr_type) { + if (expr_type == ExprType::DATA) { + parameter_push_back_expression(std::move(expr)); + return; + } + + expressions.push_back(std::move(expr)); + ActionParam param; + switch (expr_type) { + case ExprType::HEADER: + param.tag = ActionParam::EXPRESSION_HEADER; + break; + case ExprType::HEADER_STACK: + param.tag = ActionParam::EXPRESSION_HEADER_STACK; + break; + case ExprType::UNION: + param.tag = ActionParam::EXPRESSION_HEADER_UNION; + break; + case ExprType::UNION_STACK: + param.tag = ActionParam::EXPRESSION_HEADER_UNION_STACK; + break; + default: + assert(0 && "Invalid expression type"); + return; + } + param.expression = {static_cast(-1), expressions.back().get()}; + params.push_back(param); +} + void ActionFn::parameter_push_back_extern_instance(ExternType *extern_instance) { ActionParam param; diff --git a/src/bm_sim/expressions.cpp b/src/bm_sim/expressions.cpp index 5fd62e911..8c22de41c 100644 --- a/src/bm_sim/expressions.cpp +++ b/src/bm_sim/expressions.cpp @@ -1,4 +1,5 @@ -/* Copyright 2013-present Barefoot Networks, Inc. +/* Copyright 2013-2019 Barefoot Networks, Inc. + * Copyright 2019 VMware, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +15,7 @@ */ /* - * Antonin Bas (antonin@barefootnetworks.com) + * Antonin Bas * */ @@ -102,6 +103,112 @@ ExprOpcodesMap::get_opcode(std::string expr_name) { return instance->opcodes_map[expr_name]; } +/* static */ ExprOpcode +ExprOpcodesUtils::get_eq_opcode(ExprType expr_type) { + switch (expr_type) { + case ExprType::DATA: + return ExprOpcode::EQ_DATA; + case ExprType::HEADER: + return ExprOpcode::EQ_HEADER; + case ExprType::BOOL: + return ExprOpcode::EQ_BOOL; + case ExprType::UNION: + return ExprOpcode::EQ_UNION; + default: + break; + } + assert(0); + return ExprOpcode::EQ_DATA; +} + +/* static */ ExprOpcode +ExprOpcodesUtils::get_neq_opcode(ExprType expr_type) { + switch (expr_type) { + case ExprType::DATA: + return ExprOpcode::NEQ_DATA; + case ExprType::HEADER: + return ExprOpcode::NEQ_HEADER; + case ExprType::BOOL: + return ExprOpcode::NEQ_BOOL; + case ExprType::UNION: + return ExprOpcode::NEQ_UNION; + default: + break; + } + assert(0); + return ExprOpcode::NEQ_DATA; +} + +/* static */ +ExprType +ExprOpcodesUtils::get_opcode_type(ExprOpcode opcode) { + switch (opcode) { + case ExprOpcode::LOAD_FIELD: + case ExprOpcode::LOAD_CONST: + case ExprOpcode::LOAD_LOCAL: + case ExprOpcode::LOAD_REGISTER_REF: + case ExprOpcode::LOAD_REGISTER_GEN: + case ExprOpcode::LOAD_LAST_HEADER_STACK_FIELD: + case ExprOpcode::ADD: + case ExprOpcode::SUB: + case ExprOpcode::MOD: + case ExprOpcode::DIV: + case ExprOpcode::MUL: + case ExprOpcode::SHIFT_LEFT: + case ExprOpcode::SHIFT_RIGHT: + case ExprOpcode::BIT_AND: + case ExprOpcode::BIT_OR: + case ExprOpcode::BIT_XOR: + case ExprOpcode::BIT_NEG: + case ExprOpcode::TWO_COMP_MOD: + case ExprOpcode::USAT_CAST: + case ExprOpcode::SAT_CAST: + case ExprOpcode::BOOL_TO_DATA: + case ExprOpcode::LAST_STACK_INDEX: + case ExprOpcode::SIZE_STACK: + case ExprOpcode::ACCESS_FIELD: + return ExprType::DATA; + case ExprOpcode::LOAD_BOOL: + case ExprOpcode::EQ_DATA: + case ExprOpcode::NEQ_DATA: + case ExprOpcode::GT_DATA: + case ExprOpcode::LT_DATA: + case ExprOpcode::GET_DATA: + case ExprOpcode::LET_DATA: + case ExprOpcode::EQ_HEADER: + case ExprOpcode::NEQ_HEADER: + case ExprOpcode::EQ_UNION: + case ExprOpcode::NEQ_UNION: + case ExprOpcode::EQ_BOOL: + case ExprOpcode::NEQ_BOOL: + case ExprOpcode::AND: + case ExprOpcode::OR: + case ExprOpcode::NOT: + case ExprOpcode::VALID_HEADER: + case ExprOpcode::VALID_UNION: + case ExprOpcode::DATA_TO_BOOL: + return ExprType::BOOL; + case ExprOpcode::LOAD_HEADER: + case ExprOpcode::DEREFERENCE_HEADER_STACK: + case ExprOpcode::ACCESS_UNION_HEADER: + return ExprType::HEADER; + case ExprOpcode::LOAD_HEADER_STACK: + return ExprType::HEADER_STACK; + case ExprOpcode::LOAD_UNION: + case ExprOpcode::DEREFERENCE_UNION_STACK: + return ExprType::UNION; + case ExprOpcode::LOAD_UNION_STACK: + return ExprType::UNION_STACK; + case ExprOpcode::TERNARY_OP: + return ExprType::UNKNOWN; + case ExprOpcode::SKIP: + break; + } + assert(0); + return ExprType::UNKNOWN; +} + + Expression::Expression() { // trick so that empty expressions can still be executed build(); @@ -291,64 +398,110 @@ Expression::grab_register_accesses(RegisterSync *register_sync) const { } } -/* I have made this function more efficient by using thread_local variables - instead of dynamic allocation at each call. Maybe it would be better to just - try to use a stack allocator */ -void -Expression::eval_(const PHV &phv, ExprType expr_type, - const std::vector &locals, - bool *b_res, Data *d_res) const { - assert(built); +struct ExpressionTemps { + ExpressionTemps() + : data_temps_size(4), data_temps(data_temps_size) { } - if (ops.empty()) { - // special case, where the expression is empty - // not sure if this is the best way to handle this case, maybe the compiler - // should make sure this never happens instead and we should treat this as - // an error - switch (expr_type) { - case ExprType::EXPR_BOOL: - *b_res = false; - return; - case ExprType::EXPR_DATA: - d_res->set(0); - return; + void prepare(int data_registers_cnt) { + while (data_temps_size < data_registers_cnt) { + data_temps.emplace_back(); + data_temps_size++; } + + bool_temps_stack.clear(); + data_temps_stack.clear(); + header_temps_stack.clear(); + stack_temps_stack.clear(); + union_temps_stack.clear(); + } + + void push_bool(bool b) { + bool_temps_stack.push_back(b); + } + + bool pop_bool() { + auto r = bool_temps_stack.back(); + bool_temps_stack.pop_back(); + return r; + } + + void push_data(const Data *data) { + data_temps_stack.push_back(data); + } + + const Data *pop_data() { + auto *r = data_temps_stack.back(); + data_temps_stack.pop_back(); + return r; + } + + void push_header(const Header *hdr) { + header_temps_stack.push_back(hdr); + } + + const Header *pop_header() { + auto *r = header_temps_stack.back(); + header_temps_stack.pop_back(); + return r; + } + + void push_stack(const StackIface *stack) { + stack_temps_stack.push_back(stack); + } + + const StackIface *pop_stack() { + auto *r = stack_temps_stack.back(); + stack_temps_stack.pop_back(); + return r; + } + + const HeaderStack *pop_header_stack() { + return static_cast(pop_stack()); + } + + const HeaderUnionStack *pop_union_stack() { + return static_cast(pop_stack()); } - static thread_local int data_temps_size = 4; - // std::vector data_temps(data_registers_cnt); - static thread_local std::vector data_temps(data_temps_size); - while (data_temps_size < data_registers_cnt) { - data_temps.emplace_back(); - data_temps_size++; + void push_union(const HeaderUnion *hdr_union) { + union_temps_stack.push_back(hdr_union); } - /* Logically, I am using these as stacks but experiments showed that using - vectors directly was more efficient (also I can call reserve to avoid - multiple calls to malloc */ + const HeaderUnion *pop_union() { + auto *r = union_temps_stack.back(); + union_temps_stack.pop_back(); + return r; + } - /* 4 is arbitrary, it is possible to do an analysis on the Expression to find - the exact number needed, but I don't think it is worth it... */ + static ExpressionTemps &get_instance() { + // using a static thread-local variable to avoid allocation new memory every + // time an expression needs to be evaluated. An alternative could be a + // custom stack allocator. + static thread_local ExpressionTemps instance; + return instance; + } - static thread_local std::vector bool_temps_stack; - bool_temps_stack.clear(); - // bool_temps_stack.reserve(4); + int data_temps_size; + std::vector data_temps; - static thread_local std::vector data_temps_stack; - data_temps_stack.clear(); - // data_temps_stack.reserve(4); + // Logically, I am using these as stacks but experiments showed that using + // vectors directly was more efficient. + std::vector bool_temps_stack; + std::vector data_temps_stack; + std::vector header_temps_stack; + std::vector stack_temps_stack; + std::vector union_temps_stack; +}; - static thread_local std::vector header_temps_stack; - header_temps_stack.clear(); - // header_temps_stack.reserve(4); +void +Expression::eval_(const PHV &phv, + const std::vector &locals, + ExpressionTemps *temps) const { + assert(built); - static thread_local std::vector stack_temps_stack; - stack_temps_stack.clear(); - // stack_temps_stack.reserve(2); + temps->prepare(data_registers_cnt); - static thread_local std::vector union_temps_stack; - union_temps_stack.clear(); - // union_temps_stack.reserve(2); + auto &data_temps = temps->data_temps; bool lb, rb; const Data *ld, *rd; @@ -361,245 +514,241 @@ Expression::eval_(const PHV &phv, ExprType expr_type, const auto &op = ops[i]; switch (op.opcode) { case ExprOpcode::LOAD_FIELD: - data_temps_stack.push_back( - &(phv.get_field(op.field.header, op.field.field_offset))); + temps->push_data( + &phv.get_field(op.field.header, op.field.field_offset)); break; case ExprOpcode::LOAD_HEADER: - header_temps_stack.push_back(&(phv.get_header(op.header))); + temps->push_header(&phv.get_header(op.header)); break; case ExprOpcode::LOAD_HEADER_STACK: - stack_temps_stack.push_back(&(phv.get_header_stack(op.header_stack))); + temps->push_stack(&phv.get_header_stack(op.header_stack)); break; case ExprOpcode::LOAD_LAST_HEADER_STACK_FIELD: - data_temps_stack.push_back( - &(phv.get_header_stack(op.stack_field.header_stack).get_last() - .get_field(op.stack_field.field_offset))); + temps->push_data( + &phv.get_header_stack(op.stack_field.header_stack).get_last() + .get_field(op.stack_field.field_offset)); break; case ExprOpcode::LOAD_UNION: - union_temps_stack.push_back(&(phv.get_header_union(op.header_union))); + temps->push_union(&phv.get_header_union(op.header_union)); break; case ExprOpcode::LOAD_UNION_STACK: - stack_temps_stack.push_back( - &(phv.get_header_union_stack(op.header_union_stack))); + temps->push_stack(&phv.get_header_union_stack(op.header_union_stack)); break; case ExprOpcode::LOAD_BOOL: - bool_temps_stack.push_back(op.bool_value); + temps->push_bool(op.bool_value); break; case ExprOpcode::LOAD_CONST: - data_temps_stack.push_back(&const_values[op.const_offset]); + temps->push_data(&const_values[op.const_offset]); break; case ExprOpcode::LOAD_LOCAL: - data_temps_stack.push_back(&locals[op.local_offset]); + temps->push_data(&locals[op.local_offset]); break; case ExprOpcode::LOAD_REGISTER_REF: - data_temps_stack.push_back( - &op.register_ref.array->at(op.register_ref.idx)); + temps->push_data(&op.register_ref.array->at(op.register_ref.idx)); break; case ExprOpcode::LOAD_REGISTER_GEN: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - data_temps_stack.push_back(&op.register_array->at(rd->get())); + rd = temps->pop_data(); + temps->push_data(&op.register_array->at(rd->get())); break; case ExprOpcode::ACCESS_FIELD: - data_temps_stack.push_back( - &(header_temps_stack.back()->get_field(op.field_offset))); - header_temps_stack.pop_back(); + rh = temps->pop_header(); + temps->push_data(&rh->get_field(op.field_offset)); break; case ExprOpcode::ACCESS_UNION_HEADER: - ru = union_temps_stack.back(); union_temps_stack.pop_back(); - header_temps_stack.push_back(&ru->at(op.header_offset)); + ru = temps->pop_union(); + temps->push_header(&ru->at(op.header_offset)); break; case ExprOpcode::ADD: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].add(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::SUB: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].sub(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::MOD: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].mod(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::DIV: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].divide(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::MUL: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].multiply(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::SHIFT_LEFT: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].shift_left(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::SHIFT_RIGHT: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].shift_right(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::EQ_DATA: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); - bool_temps_stack.push_back(*ld == *rd); + rd = temps->pop_data(); + ld = temps->pop_data(); + temps->push_bool(*ld == *rd); break; case ExprOpcode::NEQ_DATA: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); - bool_temps_stack.push_back(*ld != *rd); + rd = temps->pop_data(); + ld = temps->pop_data(); + temps->push_bool(*ld != *rd); break; case ExprOpcode::GT_DATA: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); - bool_temps_stack.push_back(*ld > *rd); + rd = temps->pop_data(); + ld = temps->pop_data(); + temps->push_bool(*ld > *rd); break; case ExprOpcode::LT_DATA: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); - bool_temps_stack.push_back(*ld < *rd); + rd = temps->pop_data(); + ld = temps->pop_data(); + temps->push_bool(*ld < *rd); break; case ExprOpcode::GET_DATA: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); - bool_temps_stack.push_back(*ld >= *rd); + rd = temps->pop_data(); + ld = temps->pop_data(); + temps->push_bool(*ld >= *rd); break; case ExprOpcode::LET_DATA: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); - bool_temps_stack.push_back(*ld <= *rd); + rd = temps->pop_data(); + ld = temps->pop_data(); + temps->push_bool(*ld <= *rd); break; case ExprOpcode::EQ_HEADER: - rh = header_temps_stack.back(); header_temps_stack.pop_back(); - lh = header_temps_stack.back(); header_temps_stack.pop_back(); - bool_temps_stack.push_back(lh->cmp(*rh)); + rh = temps->pop_header(); + lh = temps->pop_header(); + temps->push_bool(lh->cmp(*rh)); break; case ExprOpcode::NEQ_HEADER: - rh = header_temps_stack.back(); header_temps_stack.pop_back(); - lh = header_temps_stack.back(); header_temps_stack.pop_back(); - bool_temps_stack.push_back(!lh->cmp(*rh)); + rh = temps->pop_header(); + lh = temps->pop_header(); + temps->push_bool(!lh->cmp(*rh)); break; case ExprOpcode::EQ_UNION: - ru = union_temps_stack.back(); union_temps_stack.pop_back(); - lu = union_temps_stack.back(); union_temps_stack.pop_back(); - bool_temps_stack.push_back(lu->cmp(*ru)); + ru = temps->pop_union(); + lu = temps->pop_union(); + temps->push_bool(lu->cmp(*ru)); break; case ExprOpcode::NEQ_UNION: - ru = union_temps_stack.back(); union_temps_stack.pop_back(); - lu = union_temps_stack.back(); union_temps_stack.pop_back(); - bool_temps_stack.push_back(!lu->cmp(*ru)); + ru = temps->pop_union(); + lu = temps->pop_union(); + temps->push_bool(!lu->cmp(*ru)); break; case ExprOpcode::EQ_BOOL: - rb = bool_temps_stack.back(); bool_temps_stack.pop_back(); - lb = bool_temps_stack.back(); bool_temps_stack.pop_back(); - bool_temps_stack.push_back(lb == rb); + rb = temps->pop_bool(); + lb = temps->pop_bool(); + temps->push_bool(lb == rb); break; case ExprOpcode::NEQ_BOOL: - rb = bool_temps_stack.back(); bool_temps_stack.pop_back(); - lb = bool_temps_stack.back(); bool_temps_stack.pop_back(); - bool_temps_stack.push_back(lb != rb); + rb = temps->pop_bool(); + lb = temps->pop_bool(); + temps->push_bool(lb != rb); break; case ExprOpcode::AND: - rb = bool_temps_stack.back(); bool_temps_stack.pop_back(); - lb = bool_temps_stack.back(); bool_temps_stack.pop_back(); - bool_temps_stack.push_back(lb && rb); + rb = temps->pop_bool(); + lb = temps->pop_bool(); + temps->push_bool(lb && rb); break; case ExprOpcode::OR: - rb = bool_temps_stack.back(); bool_temps_stack.pop_back(); - lb = bool_temps_stack.back(); bool_temps_stack.pop_back(); - bool_temps_stack.push_back(lb || rb); + rb = temps->pop_bool(); + lb = temps->pop_bool(); + temps->push_bool(lb || rb); break; case ExprOpcode::NOT: - rb = bool_temps_stack.back(); bool_temps_stack.pop_back(); - bool_temps_stack.push_back(!rb); + rb = temps->pop_bool(); + temps->push_bool(!rb); break; case ExprOpcode::BIT_AND: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].bit_and(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::BIT_OR: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].bit_or(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::BIT_XOR: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].bit_xor(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::BIT_NEG: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); data_temps[op.data_dest_index].bit_neg(*rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::VALID_HEADER: - bool_temps_stack.push_back(header_temps_stack.back()->is_valid()); - header_temps_stack.pop_back(); + rh = temps->pop_header(); + temps->push_bool(rh->is_valid()); break; case ExprOpcode::VALID_UNION: - bool_temps_stack.push_back(union_temps_stack.back()->is_valid()); - union_temps_stack.pop_back(); + ru = temps->pop_union(); + temps->push_bool(ru->is_valid()); break; case ExprOpcode::TERNARY_OP: - if (bool_temps_stack.back()) - i += 1; - bool_temps_stack.pop_back(); + rb = temps->pop_bool(); + if (rb) i += 1; break; case ExprOpcode::SKIP: @@ -607,65 +756,61 @@ Expression::eval_(const PHV &phv, ExprType expr_type, break; case ExprOpcode::TWO_COMP_MOD: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].two_comp_mod(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::USAT_CAST: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].usat_cast(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::SAT_CAST: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - ld = data_temps_stack.back(); data_temps_stack.pop_back(); + rd = temps->pop_data(); + ld = temps->pop_data(); data_temps[op.data_dest_index].sat_cast(*ld, *rd); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::DATA_TO_BOOL: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - bool_temps_stack.push_back(!rd->test_eq(0)); + rd = temps->pop_data(); + temps->push_bool(!rd->test_eq(0)); break; case ExprOpcode::BOOL_TO_DATA: - rb = bool_temps_stack.back(); bool_temps_stack.pop_back(); + rb = temps->pop_bool(); data_temps[op.data_dest_index].set(static_cast(rb)); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::DEREFERENCE_HEADER_STACK: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - hs = static_cast(stack_temps_stack.back()); - header_temps_stack.push_back(&hs->at(rd->get())); - stack_temps_stack.pop_back(); + rd = temps->pop_data(); + hs = temps->pop_header_stack(); + temps->push_header(&hs->at(rd->get())); break; // LAST_STACK_INDEX seems a little redundant given SIZE_STACK, but I don't // exclude in the future to do some sanity checking for LAST_STACK_INDEX case ExprOpcode::LAST_STACK_INDEX: - data_temps[op.data_dest_index].set( - stack_temps_stack.back()->get_count() - 1); - stack_temps_stack.pop_back(); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + hs = temps->pop_header_stack(); + data_temps[op.data_dest_index].set(hs->get_count() - 1); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::SIZE_STACK: - data_temps[op.data_dest_index].set( - stack_temps_stack.back()->get_count()); - stack_temps_stack.pop_back(); - data_temps_stack.push_back(&data_temps[op.data_dest_index]); + hs = temps->pop_header_stack(); + data_temps[op.data_dest_index].set(hs->get_count()); + temps->push_data(&data_temps[op.data_dest_index]); break; case ExprOpcode::DEREFERENCE_UNION_STACK: - rd = data_temps_stack.back(); data_temps_stack.pop_back(); - hus = static_cast(stack_temps_stack.back()); - union_temps_stack.push_back(&hus->at(rd->get())); - stack_temps_stack.pop_back(); + rd = temps->pop_data(); + hus = temps->pop_union_stack(); + temps->push_union(&hus->at(rd->get())); break; default: @@ -673,35 +818,86 @@ Expression::eval_(const PHV &phv, ExprType expr_type, break; } } - - switch (expr_type) { - case ExprType::EXPR_BOOL: - *b_res = bool_temps_stack.back(); - break; - case ExprType::EXPR_DATA: - d_res->set(*(data_temps_stack.back())); - break; - } } bool Expression::eval_bool(const PHV &phv, const std::vector &locals) const { - bool result; - eval_(phv, ExprType::EXPR_BOOL, locals, &result, nullptr); - return result; + // special case, where the expression is empty + // not sure if this is the best way to handle this case, maybe the compiler + // should make sure this never happens instead and we should treat this as + // an error + if (ops.empty()) return false; + auto &temps = ExpressionTemps::get_instance(); + eval_(phv, locals, &temps); + return temps.pop_bool(); } Data Expression::eval_arith(const PHV &phv, const std::vector &locals) const { - Data result_ptr; - eval_(phv, ExprType::EXPR_DATA, locals, nullptr, &result_ptr); - return result_ptr; + if (ops.empty()) return Data(0); + auto &temps = ExpressionTemps::get_instance(); + eval_(phv, locals, &temps); + return *temps.pop_data(); } void Expression::eval_arith(const PHV &phv, Data *data, const std::vector &locals) const { - eval_(phv, ExprType::EXPR_DATA, locals, nullptr, data); + if (ops.empty()) { + data->set(0); + return; + } + auto &temps = ExpressionTemps::get_instance(); + eval_(phv, locals, &temps); + data->set(*temps.pop_data()); +} + +// Unfortunately all the methods use a const_cast. I wanted to avoid having to +// change the interface for existing methods (eval_arith and eval_bool), for +// which the expectation is that the PHV will not be modified during +// evaluation. This meant that changing the private eval_ method was +// difficult. I haven't found a good solution, which doesn't make the code more +// complex. yet. + +Data & +Expression::eval_arith_lvalue(PHV *phv, const std::vector &locals) const { + assert(!ops.empty()); + auto &temps = ExpressionTemps::get_instance(); + eval_(*phv, locals, &temps); + return const_cast(*temps.pop_data()); +} + +Header & +Expression::eval_header(PHV *phv, const std::vector &locals) const { + assert(!ops.empty()); + auto &temps = ExpressionTemps::get_instance(); + eval_(*phv, locals, &temps); + return const_cast
(*temps.pop_header()); +} + +HeaderStack & +Expression::eval_header_stack(PHV *phv, const std::vector &locals) const { + assert(!ops.empty()); + auto &temps = ExpressionTemps::get_instance(); + eval_(*phv, locals, &temps); + return const_cast(*temps.pop_header_stack()); +} + +HeaderUnion & +Expression::eval_header_union(PHV *phv, const std::vector &locals) const { + assert(!ops.empty()); + auto &temps = ExpressionTemps::get_instance(); + eval_(*phv, locals, &temps); + return const_cast(*temps.pop_union()); +} + +HeaderUnionStack & +Expression::eval_header_union_stack( + PHV *phv, const std::vector &locals) const { + assert(!ops.empty()); + auto &temps = ExpressionTemps::get_instance(); + eval_(*phv, locals, &temps); + return const_cast(*temps.pop_union_stack()); } // TODO(antonin): If there is a ternary op, we will over-estimate this number, diff --git a/tests/test_actions.cpp b/tests/test_actions.cpp index c4c586109..a9bada768 100644 --- a/tests/test_actions.cpp +++ b/tests/test_actions.cpp @@ -1,4 +1,5 @@ -/* Copyright 2013-present Barefoot Networks, Inc. +/* Copyright 2013-2019 Barefoot Networks, Inc. + * Copyright 2019 VMware, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +15,7 @@ */ /* - * Antonin Bas (antonin@barefootnetworks.com) + * Antonin Bas * */ @@ -632,6 +633,47 @@ TEST_F(ActionsTest, HeaderUnion) { EXPECT_EQ("test_union", primitive.get_union_name()); } +TEST_F(ActionsTest, ExpressionHeader) { + RemoveHeader primitive; + testActionFn.push_back_primitive(&primitive); + + std::unique_ptr expr(new Expression()); + expr->push_back_load_header(testHeader2); + expr->build(); + testActionFn.parameter_push_back_expression( + std::move(expr), ExprType::HEADER); + + auto &hdr2 = phv->get_header(testHeader2); + hdr2.mark_valid(); + ASSERT_TRUE(hdr2.is_valid()); + + testActionFnEntry(pkt.get()); + EXPECT_FALSE(hdr2.is_valid()); +} + +TEST_F(ActionsTest, ExpressionDataLV) { + const uint64_t v = 12345u; + + Set primitive; + testActionFn.push_back_primitive(&primitive); + + std::unique_ptr expr(new Expression()); + expr->push_back_load_header_stack(testHeaderStack); + expr->push_back_load_const(Data(1)); // testHeaderS1 + expr->push_back_op(ExprOpcode::DEREFERENCE_HEADER_STACK); + expr->push_back_access_field(3); // f16 + expr->build(); + testActionFn.parameter_push_back_expression(std::move(expr), ExprType::DATA); + + testActionFn.parameter_push_back_const(Data(v)); + + const auto &f = phv->get_field(testHeaderS1, 3); // testHeaderS1.f16 + ASSERT_NE(f.get(), v); + + testActionFnEntry(pkt.get()); + EXPECT_EQ(f.get(), v); +} + TEST_F(ActionsTest, Jump) { auto primitive_if = ActionOpcodesMap::get_instance()->get_primitive( "_jump_if_zero"); diff --git a/tests/test_conditionals.cpp b/tests/test_conditionals.cpp index 540a09619..dd4b6b43b 100644 --- a/tests/test_conditionals.cpp +++ b/tests/test_conditionals.cpp @@ -1,4 +1,5 @@ -/* Copyright 2013-present Barefoot Networks, Inc. +/* Copyright 2013-2019 Barefoot Networks, Inc. + * Copyright 2019 VMware, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +15,7 @@ */ /* - * Antonin Bas (antonin@barefootnetworks.com) + * Antonin Bas * */ @@ -46,7 +47,6 @@ class ConditionalsTest : public ::testing::Test { header_id_t testHeader1{0}, testHeader2{1}; header_stack_id_t testHeaderStack{0}; - size_t stack_depth{2}; ConditionalsTest() : testHeaderType("test_t", 0) { @@ -793,7 +793,6 @@ class ConditionalsUnionTest : public ConditionalsTest { header_union_id_t testHeaderUnion1{1}; header_union_stack_id_t testHeaderUnionStack{0}; - size_t union_stack_depth{2}; ConditionalsUnionTest() { phv_factory.push_back_header("test3", testHeader3, testHeaderType); diff --git a/tests/test_expressions.cpp b/tests/test_expressions.cpp index faf141662..c4715a831 100644 --- a/tests/test_expressions.cpp +++ b/tests/test_expressions.cpp @@ -1,4 +1,5 @@ -/* Copyright 2013-present Barefoot Networks, Inc. +/* Copyright 2013-2019 Barefoot Networks, Inc. + * Copyright 2019 VMware, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +15,7 @@ */ /* - * Antonin Bas (antonin@barefootnetworks.com) + * Antonin Bas * */ @@ -36,6 +37,8 @@ class ExpressionsTest : public ::testing::Test { HeaderType testHeaderType; header_id_t testHeader1{0}, testHeader2{1}; + header_stack_id_t testHeaderStack{0}; + ExpressionsTest() : testHeaderType("test_t", 0) { testHeaderType.push_back_field("f32", 32); @@ -45,6 +48,10 @@ class ExpressionsTest : public ::testing::Test { testHeaderType.push_back_field("f128", 128); phv_factory.push_back_header("test1", testHeader1, testHeaderType); phv_factory.push_back_header("test2", testHeader2, testHeaderType); + + phv_factory.push_back_header_stack( + "test_stack", testHeaderStack, testHeaderType, + {testHeader1, testHeader2}); } virtual void SetUp() { @@ -59,7 +66,7 @@ TEST_F(ExpressionsTest, EmptyArith) { expr.build(); ASSERT_TRUE(expr.empty()); const auto data = expr.eval_arith(*phv.get()); - ASSERT_EQ(0, data.get()); + EXPECT_EQ(0, data.get()); } TEST_F(ExpressionsTest, EmptyBool) { @@ -67,5 +74,37 @@ TEST_F(ExpressionsTest, EmptyBool) { expr.build(); ASSERT_TRUE(expr.empty()); const auto b = expr.eval_bool(*phv.get()); - ASSERT_FALSE(b); + EXPECT_FALSE(b); +} + +// TODO(antonin): test the union variants + +TEST_F(ExpressionsTest, HeaderRef) { + Expression expr; + expr.push_back_load_header(testHeader1); + expr.build(); + + auto &hdr = expr.eval_header(phv.get()); + EXPECT_EQ(&phv->get_header(testHeader1), &hdr); +} + +TEST_F(ExpressionsTest, HeaderStackRef) { + Expression expr; + expr.push_back_load_header_stack(testHeaderStack); + expr.build(); + + auto &stack = expr.eval_header_stack(phv.get()); + EXPECT_EQ(&phv->get_header_stack(testHeaderStack), &stack); +} + +TEST_F(ExpressionsTest, FieldRef) { + Expression expr; + expr.push_back_load_header_stack(testHeaderStack); + expr.push_back_load_const(Data(1)); // testHeader2 + expr.push_back_op(ExprOpcode::DEREFERENCE_HEADER_STACK); + expr.push_back_access_field(3); // f16 + expr.build(); + + auto &data = expr.eval_arith_lvalue(phv.get()); + EXPECT_EQ(&phv->get_field(testHeader2, 3), &data); }