Split sim deps out of ir_test_base

Most tests have no need of verilog simulation so there's no need to pull in this large dependency for every one of them. Instead create a 'sim_test_base' for those targets that actually want to simulate verilog directly. PiperOrigin-RevId: 672639206
google · Sep 9, 2024 · 65dcd0e · 65dcd0e
1 parent 60bc976
commit 65dcd0e
Show file tree

Hide file tree

Showing 17 changed files with 373 additions and 208 deletions.
diff --git a/third_party/xls_colors/BUILD b/third_party/xls_colors/BUILD
@@ -49,8 +49,8 @@ cc_test(
         "//xls/dslx/ir_convert:convert_options",
         "//xls/dslx/ir_convert:ir_converter",
         "//xls/ir:bits",
-        "//xls/ir:ir_test_base",
         "//xls/ir:value",
+        "//xls/simulation:sim_test_base",
         "@com_google_googletest//:gtest",
     ],
 )

diff --git a/third_party/xls_colors/test.cc b/third_party/xls_colors/test.cc
@@ -25,7 +25,6 @@
 #include <string>
 
 #include "gtest/gtest.h"
-#include "third_party/xls_colors/fast_hsv2rgb.h"
 #include "xls/common/file/filesystem.h"
 #include "xls/common/file/get_runfile_path.h"
 #include "xls/common/status/matchers.h"
@@ -35,21 +34,23 @@
 #include "xls/dslx/ir_convert/ir_converter.h"
 #include "xls/dslx/parse_and_typecheck.h"
 #include "xls/ir/bits.h"
-#include "xls/ir/ir_test_base.h"
 #include "xls/ir/value.h"
+#include "xls/simulation/sim_test_base.h"
+#include "third_party/xls_colors/fast_hsv2rgb.h"
 
 namespace {
 
 using xls::UBits;
 using xls::Value;
 
-class XlsColorsTest : public xls::IrTestBase {
+class XlsColorsTest : public xls::SimTestBase {
  public:
   void Run(uint16_t hstart, uint16_t hlimit) {
     XLS_ASSERT_OK_AND_ASSIGN(
         std::filesystem::path path,
         xls::GetXlsRunfilePath("third_party/xls_colors/hsv2rgb.x"));
-    XLS_ASSERT_OK_AND_ASSIGN(std::string moduleText, xls::GetFileContents(path));
+    XLS_ASSERT_OK_AND_ASSIGN(std::string moduleText,
+                             xls::GetFileContents(path));
     auto import_data = xls::dslx::CreateImportDataForTest();
     XLS_ASSERT_OK_AND_ASSIGN(
         xls::dslx::TypecheckedModule tm,

diff --git a/xls/contrib/xlscc/unit_tests/BUILD b/xls/contrib/xlscc/unit_tests/BUILD
@@ -48,11 +48,11 @@ cc_library(
         "//xls/ir",
         "//xls/ir:bits",
         "//xls/ir:events",
-        "//xls/ir:ir_test_base",
         "//xls/ir:op",
         "//xls/ir:source_location",
         "//xls/ir:value",
         "//xls/ir:value_utils",
+        "//xls/simulation:sim_test_base",
         "@com_google_absl//absl/base:log_severity",
         "@com_google_absl//absl/container:btree",
         "@com_google_absl//absl/container:flat_hash_map",

diff --git a/xls/contrib/xlscc/unit_tests/unit_test.h b/xls/contrib/xlscc/unit_tests/unit_test.h
@@ -39,11 +39,11 @@
 #include "xls/contrib/xlscc/translator.h"
 #include "xls/ir/bits.h"
 #include "xls/ir/events.h"
-#include "xls/ir/ir_test_base.h"
 #include "xls/ir/node.h"
 #include "xls/ir/proc.h"
 #include "xls/ir/source_location.h"
 #include "xls/ir/value.h"
+#include "xls/simulation/sim_test_base.h"
 
 struct CapturedLogEntry {
   CapturedLogEntry();
@@ -60,7 +60,7 @@ struct CapturedLogEntry {
 
 // Support for XLS[cc] related tests, such as invoking XLS[cc]
 //  with the appropriate parameters for the test environment
-class XlsccTestBase : public xls::IrTestBase, public ::absl::LogSink {
+class XlsccTestBase : public xls::SimTestBase, public ::absl::LogSink {
  public:
   XlsccTestBase();
 

diff --git a/xls/ir/BUILD b/xls/ir/BUILD
@@ -891,27 +891,15 @@ cc_library(
     hdrs = ["ir_test_base.h"],
     deps = [
         ":bits",
-        ":events",
         ":ir",
         ":ir_parser",
         ":op",
         ":type",
         ":value",
-        ":value_test_util",
         ":verifier",
-        "//xls/codegen:codegen_options",
-        "//xls/codegen:combinational_generator",
-        "//xls/codegen:module_signature",
-        "//xls/codegen/vast",
-        "//xls/common:source_location",
-        "//xls/common/status:matchers",
         "//xls/common/status:ret_check",
         "//xls/common/status:status_macros",
         "//xls/estimators/delay_model:delay_estimator",
-        "//xls/interpreter:ir_interpreter",
-        "//xls/passes:optimization_pass_pipeline",
-        "//xls/simulation:default_verilog_simulator",
-        "//xls/simulation:module_simulator",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/log",
         "@com_google_absl//absl/status",

diff --git a/xls/ir/ir_test_base.cc b/xls/ir/ir_test_base.cc
@@ -27,27 +27,15 @@
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/str_format.h"
-#include "xls/codegen/codegen_options.h"
-#include "xls/codegen/combinational_generator.h"
-#include "xls/codegen/module_signature.h"
-#include "xls/codegen/vast/vast.h"
-#include "xls/common/source_location.h"
-#include "xls/common/status/matchers.h"
 #include "xls/common/status/ret_check.h"
 #include "xls/common/status/status_macros.h"
-#include "xls/interpreter/function_interpreter.h"
 #include "xls/ir/bits.h"
-#include "xls/ir/events.h"
 #include "xls/ir/function_base.h"
 #include "xls/ir/ir_parser.h"
 #include "xls/ir/nodes.h"
 #include "xls/ir/type.h"
 #include "xls/ir/value.h"
-#include "xls/ir/value_test_util.h"
 #include "xls/ir/verifier.h"
-#include "xls/passes/optimization_pass_pipeline.h"
-#include "xls/simulation/default_verilog_simulator.h"
-#include "xls/simulation/module_simulator.h"
 
 namespace xls {
 
@@ -153,57 +141,6 @@ Block* IrTestBase::FindBlock(std::string_view name, Package* package) {
   LOG(FATAL) << "No block named " << name << " in package:\n" << *package;
 }
 
-void IrTestBase::RunAndExpectEq(
-    const absl::flat_hash_map<std::string, uint64_t>& args, uint64_t expected,
-    std::string_view package_text, bool run_optimized, bool simulate,
-    xabsl::SourceLocation loc) {
-  // Emit the filename/line of the test code in any failure message. The
-  // location is captured as a default argument to RunAndExpectEq.
-  testing::ScopedTrace trace(loc.file_name(), loc.line(),
-                             "RunAndExpectEq failed");
-  VLOG(3) << "Package text:\n" << package_text;
-  XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr<Package> package,
-                           ParsePackage(package_text));
-  absl::flat_hash_map<std::string, Value> arg_values;
-  XLS_ASSERT_OK_AND_ASSIGN(arg_values, UInt64ArgsToValues(args, package.get()));
-  XLS_ASSERT_OK_AND_ASSIGN(Value expected_value,
-                           UInt64ResultToValue(expected, package.get()));
-
-  RunAndExpectEq(arg_values, expected_value, std::move(package), run_optimized,
-                 simulate);
-}
-
-void IrTestBase::RunAndExpectEq(
-    const absl::flat_hash_map<std::string, Bits>& args, Bits expected,
-    std::string_view package_text, bool run_optimized, bool simulate,
-    xabsl::SourceLocation loc) {
-  // Emit the filename/line of the test code in any failure message. The
-  // location is captured as a default argument to RunAndExpectEq.
-  testing::ScopedTrace trace(loc.file_name(), loc.line(),
-                             "RunAndExpectEq failed");
-  XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr<Package> package,
-                           ParsePackage(package_text));
-  absl::flat_hash_map<std::string, Value> args_as_values;
-  for (const auto& pair : args) {
-    args_as_values[pair.first] = Value(pair.second);
-  }
-  RunAndExpectEq(args_as_values, Value(std::move(expected)), std::move(package),
-                 run_optimized, simulate);
-}
-
-void IrTestBase::RunAndExpectEq(
-    const absl::flat_hash_map<std::string, Value>& args, Value expected,
-    std::string_view package_text, bool run_optimized, bool simulate,
-    xabsl::SourceLocation loc) {
-  // Emit the filename/line of the test code in any failure message. The
-  // location is captured as a default argument to RunAndExpectEq.
-  testing::ScopedTrace trace(loc.file_name(), loc.line(),
-                             "RunAndExpectEq failed");
-  XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr<Package> package,
-                           ParsePackage(package_text));
-  RunAndExpectEq(args, expected, std::move(package), run_optimized, simulate);
-}
-
 absl::StatusOr<absl::flat_hash_map<std::string, Value>>
 IrTestBase::UInt64ArgsToValues(
     const absl::flat_hash_map<std::string, uint64_t>& args, Package* package) {
@@ -256,61 +193,5 @@ absl::StatusOr<Value> IrTestBase::UInt64ResultToValue(uint64_t value,
   return Value(UBits(value, return_type->AsBitsOrDie()->bit_count()));
 }
 
-void IrTestBase::RunAndExpectEq(
-    const absl::flat_hash_map<std::string, Value>& args, const Value& expected,
-    std::unique_ptr<Package>&& package, bool run_optimized, bool simulate) {
-  InterpreterEvents unopt_events;
-
-  // Run interpreter on unoptimized IR.
-  {
-    XLS_ASSERT_OK_AND_ASSIGN(Function * entry, package->GetTopAsFunction());
-    XLS_ASSERT_OK_AND_ASSIGN(InterpreterResult<Value> result,
-                             InterpretFunctionKwargs(entry, args));
-    XLS_ASSERT_OK(InterpreterEventsToStatus(result.events));
-    unopt_events = result.events;
-    ASSERT_TRUE(ValuesEqual(expected, result.value))
-        << "(interpreted unoptimized IR)";
-  }
-
-  if (run_optimized) {
-    // Run main pipeline.
-    XLS_ASSERT_OK(RunOptimizationPassPipeline(package.get()));
-
-    // Run interpreter on optimized IR.
-    {
-      XLS_ASSERT_OK_AND_ASSIGN(Function * main, package->GetTopAsFunction());
-      XLS_ASSERT_OK_AND_ASSIGN(InterpreterResult<Value> result,
-                               InterpretFunctionKwargs(main, args));
-      XLS_ASSERT_OK(InterpreterEventsToStatus(result.events));
-      ASSERT_EQ(unopt_events, result.events);
-      ASSERT_TRUE(ValuesEqual(expected, result.value))
-          << "(interpreted optimized IR)";
-    }
-  }
-
-  // Emit Verilog with combinational generator and run with ModuleSimulator.
-  if (simulate) {
-    ASSERT_EQ(package->functions().size(), 1);
-    std::optional<FunctionBase*> top = package->GetTop();
-    EXPECT_TRUE(top.has_value());
-    EXPECT_TRUE(top.value()->IsFunction());
-
-    XLS_ASSERT_OK_AND_ASSIGN(
-        verilog::ModuleGeneratorResult result,
-        verilog::GenerateCombinationalModule(
-            top.value(), verilog::CodegenOptions().use_system_verilog(false)));
-
-    absl::flat_hash_map<std::string, Value> arg_set;
-    for (const auto& pair : args) {
-      arg_set.insert(pair);
-    }
-    VLOG(3) << "Verilog text:\n" << result.verilog_text;
-    verilog::ModuleSimulator simulator(result.signature, result.verilog_text,
-                                       verilog::FileType::kVerilog,
-                                       &verilog::GetDefaultVerilogSimulator());
-    XLS_ASSERT_OK_AND_ASSIGN(Value actual, simulator.RunFunction(arg_set));
-    ASSERT_TRUE(ValuesEqual(expected, actual)) << "(Verilog simulation)";
-  }
-}
 
 }  // namespace xls
diff --git a/xls/ir/ir_test_base.h b/xls/ir/ir_test_base.h
@@ -27,9 +27,7 @@
 #include "absl/status/statusor.h"
 #include "absl/strings/str_format.h"
 #include "absl/strings/str_replace.h"
-#include "xls/common/source_location.h"
 #include "xls/estimators/delay_model/delay_estimator.h"
-#include "xls/ir/bits.h"
 #include "xls/ir/function.h"
 #include "xls/ir/function_base.h"
 #include "xls/ir/node.h"
@@ -101,40 +99,7 @@ class IrTestBase : public ::testing::Test {
   static Proc* FindProc(std::string_view name, Package* package);
   static Block* FindBlock(std::string_view name, Package* package);
 
-  // Runs the given package (passed as IR text) and EXPECTs the result to equal
-  // 'expected'. Runs the package in several ways:
-  // (1) unoptimized IR through the interpreter.
-  // (2) optimized IR through the interpreter. (enabled with run_optimized)
-  // (3) pipeline generator emitted Verilog through a Verilog simulator.
-  //          (enabled with simulate)
-  static void RunAndExpectEq(
-      const absl::flat_hash_map<std::string, uint64_t>& args, uint64_t expected,
-      std::string_view package_text, bool run_optimized = true,
-      bool simulate = true,
-      xabsl::SourceLocation loc = xabsl::SourceLocation::current());
-
-  // Overload which takes Bits as arguments and the expected result.
-  static void RunAndExpectEq(
-      const absl::flat_hash_map<std::string, Bits>& args, Bits expected,
-      std::string_view package_text, bool run_optimized = true,
-      bool simulate = true,
-      xabsl::SourceLocation loc = xabsl::SourceLocation::current());
-
-  // Overload which takes Values as arguments and the expected result.
-  static void RunAndExpectEq(
-      const absl::flat_hash_map<std::string, Value>& args, Value expected,
-      std::string_view package_text, bool run_optimized = true,
-      bool simulate = true,
-      xabsl::SourceLocation loc = xabsl::SourceLocation::current());
-
- private:
-  // Helper for RunAndExpectEq which accepts arguments and expectation as Values
-  // and takes a std::unique_ptr<Package>.
-  static void RunAndExpectEq(
-      const absl::flat_hash_map<std::string, Value>& args,
-      const Value& expected, std::unique_ptr<Package>&& package,
-      bool run_optimized = true, bool simulate = true);
-
+ protected:
   // Converts the given map of uint64_t arguments into a map of Value argument
   // with the appropriate bit widths as determined by the package.
   static absl::StatusOr<absl::flat_hash_map<std::string, Value>>

diff --git a/xls/ir/ir_test_base_test.cc b/xls/ir/ir_test_base_test.cc
@@ -57,32 +57,6 @@ TEST_F(IrTestBaseTest, ValidVerifiedPackageSucceeds) {
   XLS_ASSERT_OK(ParsePackage(kTestPackage).status());
 }
 
-TEST_F(IrTestBaseTest, RunAndExpectEqRightValue) {
-  RunAndExpectEq({{"p", 3}, {"q", 10}}, 23, kTestPackage);
-}
-
-TEST_F(IrTestBaseTest, RunAndExpectEqWrongValue) {
-  EXPECT_FATAL_FAILURE(RunAndExpectEq({{"p", 3}, {"q", 10}}, 55, kTestPackage),
-                       "bits[8]:55 != bits[8]:23");
-}
-
-TEST_F(IrTestBaseTest, RunAndExpectEqArgDoesNotFit) {
-  EXPECT_FATAL_FAILURE(
-      RunAndExpectEq({{"p", 12345}, {"q", 10}}, 23, kTestPackage),
-      "Argument value 12345 for parameter 'p' does not fit in type bits[8]");
-}
-
-TEST_F(IrTestBaseTest, RunAndExpectEqExpectedResultDoesNotFit) {
-  EXPECT_FATAL_FAILURE(
-      RunAndExpectEq({{"p", 3}, {"q", 10}}, 12345, kTestPackage),
-      "Value 12345 does not fit in return type bits[8]");
-}
-
-TEST_F(IrTestBaseTest, RunAndExpectEqMissingArg) {
-  EXPECT_FATAL_FAILURE(RunAndExpectEq({{"p", 3}}, 10, kTestPackage),
-                       "Missing argument 'q'");
-}
-
 TEST_F(IrTestBaseTest, HasNodes) {
   XLS_ASSERT_OK_AND_ASSIGN(std::unique_ptr<VerifiedPackage> p,
                            ParsePackage(kTestPackage));

diff --git a/xls/modules/zstd/BUILD b/xls/modules/zstd/BUILD
@@ -231,8 +231,8 @@ cc_test(
         "//xls/dslx/ir_convert:ir_converter",
         "//xls/dslx/type_system:parametric_env",
         "//xls/ir:bits",
-        "//xls/ir:ir_test_base",
         "//xls/ir:value",
+        "//xls/simulation:sim_test_base",
         "@zstd",
         "@com_google_googletest//:gtest",
     ],

diff --git a/xls/modules/zstd/frame_header_test.cc b/xls/modules/zstd/frame_header_test.cc
@@ -13,6 +13,9 @@
 // limitations under the License.
 
 #include <string_view>
+
+// NOLINTBEGIN(build/include_order) - Silence include order warnings.
+#include "xls/simulation/sim_test_base.h"
 #define ZSTD_STATIC_LINKING_ONLY 1
 
 #include <algorithm>
@@ -23,6 +26,7 @@
 #include <string>
 #include <utility>
 #include <vector>
+// NOLINTEND(build/include_order) - Silence include order warnings.
 
 #include "gtest/gtest.h"
 #include "xls/common/fuzzing/fuzztest.h"
@@ -40,7 +44,6 @@
 #include "xls/dslx/parse_and_typecheck.h"
 #include "xls/dslx/type_system/parametric_env.h"
 #include "xls/ir/bits.h"
-#include "xls/ir/ir_test_base.h"
 #include "xls/ir/value.h"
 #include "xls/modules/zstd/data_generator.h"
 #include "external/zstd/lib/zstd.h"
@@ -91,7 +94,7 @@ class ZstdFrameHeader {
   size_t result_;
 };
 
-class FrameHeaderTest : public xls::IrTestBase {
+class FrameHeaderTest : public xls::SimTestBase {
  public:
   // Prepare simulation environment
   void SetUp() override {