Add the Tofino testgen target.

Signed-off-by: fruffy <fruffy@nyu.edu>
p4lang · fruffy · Dec 4, 2024 · Nov 25, 2024 · Dec 5, 2024 · Dec 4, 2024
commit 602ef1ea89d254a166b1e5276ecbd4108812f9ce
diff --git a/.github/workflows/ci-test-debian.yml b/.github/workflows/ci-test-debian.yml
@@ -51,6 +51,7 @@ jobs:
       BUILD_GENERATOR: Ninja
       ENABLE_GTESTS: ON
       ENABLE_TOFINO: ON
+      ENABLE_TEST_TOOLS: ON
       ENABLE_BMV2: OFF
       ENABLE_EBPF: OFF
       ENABLE_UBPF: OFF
@@ -75,6 +76,10 @@ jobs:
         run: |
           tools/ci-build.sh
 
+      - name: Run tests (Ubuntu 20.04)
+        run: ctest --output-on-failure --schedule-random -R tofino
+        working-directory: ./build
+
   # Build with GCC and test P4C on Ubuntu 20.04.
   test-ubuntu20:
     name: test-ubuntu20 (Unity ${{ matrix.unity }}, GTest ${{ matrix.gtest }})

diff --git a/backends/p4tools/common/core/abstract_execution_state.cpp b/backends/p4tools/common/core/abstract_execution_state.cpp
@@ -123,7 +123,7 @@ std::vector<const IR::Expression *> AbstractExecutionState::flattenComplexExpres
             auto subList = flattenComplexExpression(listElem->expression, flatValids);
             exprList.insert(exprList.end(), subList.begin(), subList.end());
         }
-        if (auto headerExpr = structExpr->to<IR::HeaderExpression>()) {
+        if (const auto *headerExpr = structExpr->to<IR::HeaderExpression>()) {
             flatValids.emplace_back(headerExpr->validity);
         }
     } else if (const auto *headerStackExpr = inputExpression->to<IR::HeaderStackExpression>()) {

diff --git a/backends/p4tools/modules/testgen/CMakeLists.txt b/backends/p4tools/modules/testgen/CMakeLists.txt
@@ -77,6 +77,12 @@ set(
 )
 set(TESTGEN_INCLUDES)
 
+# Only enable Tofino tests if the Tofino backend is enabled.
+# FIXME: Can we make this part of the extension?
+if (NOT ENABLE_TOFINO)
+  set (ENABLE_TOOLS_TARGET_TOFINO OFF)
+endif()
+
 file(GLOB testgen_targets RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}/targets ${CMAKE_CURRENT_SOURCE_DIR}/targets/*)
 foreach(ext ${testgen_targets})
   set(testgen_targets_dir ${CMAKE_CURRENT_SOURCE_DIR}/targets/${ext}/)

diff --git a/backends/p4tools/modules/testgen/core/small_step/expr_stepper.cpp b/backends/p4tools/modules/testgen/core/small_step/expr_stepper.cpp
@@ -131,7 +131,11 @@ bool ExprStepper::resolveMethodCallArguments(const IR::MethodCallExpression *cal
         const auto *arg = callArguments->at(idx);
         const auto *param = methodParams.at(idx);
         const auto *argExpr = arg->expression;
-        if (param->direction == IR::Direction::Out || SymbolicEnv::isSymbolicValue(argExpr)) {
+        // Avoid resolving externs because they do not actually exist in the symbolic environment.
+        // Do not resolve out parameters because we do not care about their content.
+        // Skip symbolic values since they have already been resolved.
+        if (argExpr->type->is<IR::Type_Extern>() || param->direction == IR::Direction::Out ||
+            SymbolicEnv::isSymbolicValue(argExpr)) {
             continue;
         }
         // If the parameter is not an out parameter (meaning we do not care about its content) and
@@ -510,10 +514,10 @@ bool ExprStepper::preorder(const IR::AbstractSlice *slice) {
 }
 
 void ExprStepper::stepNoMatch(std::string traceLog, const IR::Expression *condition) {
-    auto &noMatchState = condition ? state.clone() : state;
+    auto &noMatchState = (condition != nullptr) ? state.clone() : state;
     noMatchState.add(*new TraceEvents::GenericDescription("NoMatch"_cs, traceLog));
     noMatchState.replaceTopBody(Continuation::Exception::NoMatch);
-    if (condition) {
+    if (condition != nullptr) {
         result->emplace_back(condition, state, noMatchState);
     } else {
         result->emplace_back(state);

diff --git a/backends/p4tools/modules/testgen/lib/continuation.cpp b/backends/p4tools/modules/testgen/lib/continuation.cpp
@@ -110,7 +110,7 @@ Continuation::Body Continuation::apply(std::optional<const IR::Node *> value_opt
         // TODO: Resolve this in a cleaner fashion. Maybe we should not step at all?
         if (paramType->is<IR::Type_ActionEnum>()) {
             argType = paramType;
-            auto clone = valueExpr->clone();
+            auto *clone = valueExpr->clone();
             clone->type = paramType;
             value_opt = clone;
         }
@@ -121,10 +121,11 @@ Continuation::Body Continuation::apply(std::optional<const IR::Node *> value_opt
         BUG("Unexpected node passed to continuation: %1%", *value_opt);
     }
 
-    BUG_CHECK(
-        paramType->equiv(*argType),
-        "Continuation %1% has parameter of type %2%, but was given an argument %3% of type %4%",
-        (*parameterOpt), paramType, *value_opt, argType);
+    BUG_CHECK(paramType->equiv(*argType),
+              "Continuation %1% has parameter of type %2% (%3%), but was given an argument %4% of "
+              "type %5% (%6%).",
+              (*parameterOpt), paramType, paramType->node_type_name(), *value_opt, argType,
+              argType->node_type_name());
 
     // Create a copy of this continuation's body, with the value substituted for the continuation's
     // parameter.

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/expr_stepper.cpp b/backends/p4tools/modules/testgen/targets/bmv2/expr_stepper.cpp
@@ -1021,8 +1021,7 @@ const Bmv2V1ModelExprStepper::ExternMethodImpls<Bmv2V1ModelExprStepper>
                  }
              }
              bool argsAreTainted = false;
-             for (size_t idx = 0; idx < externInfo.externArguments.size(); ++idx) {
-                 const auto *arg = externInfo.externArguments.at(idx);
+             for (auto arg : externInfo.externArguments) {
                  argsAreTainted = argsAreTainted || Taint::hasTaint(arg->expression);
              }
              // If any of the input arguments is tainted, the entire extern is unreliable.
@@ -1631,7 +1630,7 @@ void Bmv2V1ModelExprStepper::evalExternMethodCall(const ExternInfo &externInfo)
     }
     // Lastly, check whether we are calling an internal extern method.
     return ExprStepper::evalExternMethodCall(externInfo);
-}  // NOLINT
+}
 
 bool Bmv2V1ModelExprStepper::preorder(const IR::P4Table *table) {
     // Delegate to the tableStepper.

diff --git a/backends/p4tools/modules/testgen/targets/tofino/CMakeLists.txt b/backends/p4tools/modules/testgen/targets/tofino/CMakeLists.txt
@@ -0,0 +1,58 @@
+if(ENABLE_TESTING)
+  # Include the test subdirectory.
+  message("-- Adding p4testgen tofino test suite")
+  include(test/P4Tests.cmake)
+endif()
+
+# Source files for p4testgen.
+set(
+TESTGEN_SOURCES
+${TESTGEN_SOURCES}
+
+${CMAKE_CURRENT_SOURCE_DIR}/test_backend/ptf.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/test_backend/stf.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/compiler_result.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/concolic.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/constants.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/hash_utils.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/map_direct_externs.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/shared_expr_stepper.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/shared_program_info.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/shared_table_stepper.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/target.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/test_backend.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/test_spec.cpp
+
+${CMAKE_CURRENT_SOURCE_DIR}/tofino/cmd_stepper.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/tofino/expr_stepper.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/tofino/program_info.cpp
+
+${CMAKE_CURRENT_SOURCE_DIR}/tofino2/cmd_stepper.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/tofino2/expr_stepper.cpp
+${CMAKE_CURRENT_SOURCE_DIR}/tofino2/program_info.cpp
+
+PARENT_SCOPE
+)
+
+# Override default settings in bf-p4c. We want those binaries to be built.
+if (NOT TARGET p4cgraphs)
+  set(ENABLE_P4C_GRAPHS ON CACHE BOOL "TRUE" FORCE)
+  add_subdirectory(${P4C_SOURCE_DIR}/backends/graphs ${P4C_BINARY_DIR}/backends/graphs)
+endif()
+
+# Ideally, we'd link against just the Barefoot midend and a minimal set of
+# transitive dependencies, but the compiler doesn't cleanly separate like
+# that. Those transitive dependencies turn out to be most of the compiler!
+set(
+TESTGEN_LIBS
+# tofinobackend
+# bfn_p4runtime
+# p4cgraphs
+# We do not use the target library variable here because top-level testgen will not find it.
+# boost_system
+${TESTGEN_LIBS}
+PARENT_SCOPE
+)
+
+# Custom IR constructs for P4Tools.
+set(IR_DEF_FILES ${IR_DEF_FILES} ${CMAKE_CURRENT_SOURCE_DIR}/testgen_tofino.def PARENT_SCOPE)
diff --git a/backends/p4tools/modules/testgen/targets/tofino/check_parser_error.h b/backends/p4tools/modules/testgen/targets/tofino/check_parser_error.h
@@ -0,0 +1,51 @@
+/*******************************************************************************
+ *  Copyright (C) 2024 Intel Corporation
+ *
+ *  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.
+ *
+ *
+ *  SPDX-License-Identifier: Apache-2.0
+ ******************************************************************************/
+
+#ifndef BACKENDS_P4TOOLS_MODULES_TESTGEN_TARGETS_TOFINO_CHECK_PARSER_ERROR_H_
+#define BACKENDS_P4TOOLS_MODULES_TESTGEN_TARGETS_TOFINO_CHECK_PARSER_ERROR_H_
+
+#include "ir/ir.h"
+
+namespace P4::P4Tools::P4Testgen::Tofino {
+
+/**
+ * Walks a given IR node to detect whether it contains a reference to the Tofino parser error.
+ * This is primarily used to detect that an ingress control references the parser error variable.
+ * If that is the case, the semantics of parser errors change in the Tofino parser. It will not
+ * drop packets and instead forward the packet to the MAU.
+ */
+class CheckParserError : public Inspector {
+ private:
+    bool parserErrorFound = false;
+
+ public:
+    CheckParserError() { setName("CheckParserError"); }
+
+    bool hasParserError() const { return parserErrorFound; }
+
+    void postorder(const IR::Member *member) override {
+        if (member->member == "parser_err") {
+            parserErrorFound = true;
+        }
+    }
+};
+
+}  // namespace P4::P4Tools::P4Testgen::Tofino
+
+#endif /* BACKENDS_P4TOOLS_MODULES_TESTGEN_TARGETS_TOFINO_CHECK_PARSER_ERROR_H_ */
diff --git a/backends/p4tools/modules/testgen/targets/tofino/compiler_result.cpp b/backends/p4tools/modules/testgen/targets/tofino/compiler_result.cpp
@@ -0,0 +1,14 @@
+#include "backends/p4tools/modules/testgen/targets/tofino/compiler_result.h"
+
+#include <utility>
+
+namespace P4::P4Tools::P4Testgen::Tofino {
+
+TofinoCompilerResult::TofinoCompilerResult(TestgenCompilerResult compilerResult,
+                                           DirectExternMap directExternMap)
+    : TestgenCompilerResult(std::move(compilerResult)),
+      directExternMap(std::move(directExternMap)) {}
+
+const DirectExternMap &TofinoCompilerResult::getDirectExternMap() const { return directExternMap; }
+
+}  // namespace P4::P4Tools::P4Testgen::Tofino
diff --git a/backends/p4tools/modules/testgen/targets/tofino/compiler_result.h b/backends/p4tools/modules/testgen/targets/tofino/compiler_result.h
@@ -0,0 +1,27 @@
+#ifndef BACKENDS_P4TOOLS_MODULES_TESTGEN_TARGETS_TOFINO_COMPILER_RESULT_H_
+#define BACKENDS_P4TOOLS_MODULES_TESTGEN_TARGETS_TOFINO_COMPILER_RESULT_H_
+
+#include "backends/p4tools/modules/testgen/core/compiler_result.h"
+#include "backends/p4tools/modules/testgen/targets/tofino/map_direct_externs.h"
+
+namespace P4::P4Tools::P4Testgen::Tofino {
+
+/// Extends the CompilerResult with information specific to the V1Model running on BMv2.
+class TofinoCompilerResult : public TestgenCompilerResult {
+ private:
+    /// The map of direct extern declarations which are attached to a table.
+    DirectExternMap directExternMap;
+
+ public:
+    explicit TofinoCompilerResult(TestgenCompilerResult compilerResult,
+                                  DirectExternMap directExternMap);
+
+    /// @returns the map of direct extern declarations which are attached to a table.
+    [[nodiscard]] const DirectExternMap &getDirectExternMap() const;
+
+    DECLARE_TYPEINFO(TofinoCompilerResult, TestgenCompilerResult);
+};
+
+}  // namespace P4::P4Tools::P4Testgen::Tofino
+
+#endif /* BACKENDS_P4TOOLS_MODULES_TESTGEN_TARGETS_TOFINO_COMPILER_RESULT_H_ */