Merge pull request google#958 from antmicro:mdudek/Fix_assert_eq_in_t…

…est_proc PiperOrigin-RevId: 533195502
kammoh · May 18, 2023 · 1f09223 · 1f09223
2 parents 6bc7853 + 1e87719
commit 1f09223
Show file tree

Hide file tree

Showing 9 changed files with 14 additions and 104 deletions.
diff --git a/xls/dslx/ir_convert/extract_conversion_order.cc b/xls/dslx/ir_convert/extract_conversion_order.cc
@@ -773,12 +773,8 @@ static absl::Status AddToReady(std::variant<Function*, TestFunction*> f,
     return absl::OkStatus();
   }
 
-  Expr* body;
-  if (std::holds_alternative<Function*>(f)) {
-    body = std::get<Function*>(f)->body();
-  } else {
-    body = std::get<TestFunction*>(f)->fn()->body();
-  }
+  // TestFunctions are covered by IsReady()
+  Expr* body = std::get<Function*>(f)->body();
   XLS_ASSIGN_OR_RETURN(const std::vector<Callee> orig_callees,
                        GetCallees(body, m, type_info, bindings, proc_id));
   XLS_VLOG(5) << "Original callees of " << std::get<Function*>(f)->identifier()
@@ -847,8 +843,7 @@ static absl::StatusOr<std::vector<ConversionRecord>> GetOrderForProc(
 }
 
 absl::StatusOr<std::vector<ConversionRecord>> GetOrder(Module* module,
-                                                       TypeInfo* type_info,
-                                                       bool traverse_tests) {
+                                                       TypeInfo* type_info) {
   XLS_CHECK_EQ(type_info->module(), module);
   std::vector<ConversionRecord> ready;
 
@@ -893,21 +888,6 @@ absl::StatusOr<std::vector<ConversionRecord>> GetOrder(Module* module,
     ready.insert(ready.end(), proc_ready.begin(), proc_ready.end());
   }
 
-  // Collect tests.
-  if (traverse_tests) {
-    for (TestFunction* test : module->GetFunctionTests()) {
-      XLS_RETURN_IF_ERROR(AddToReady(test, /*invocation=*/nullptr, module,
-                                     type_info, ParametricEnv(), &ready, {}));
-    }
-    for (TestProc* test : module->GetProcTests()) {
-      XLS_ASSIGN_OR_RETURN(TypeInfo * proc_ti,
-                           type_info->GetTopLevelProcTypeInfo(test->proc()));
-      XLS_ASSIGN_OR_RETURN(std::vector<ConversionRecord> proc_ready,
-                           GetOrderForProc(test, proc_ti, /*is_top=*/false));
-      ready.insert(ready.end(), proc_ready.begin(), proc_ready.end());
-    }
-  }
-
   // Remove duplicated functions. When performing a complete module conversion,
   // the functions and the proc are converted in that order. However, procs may
   // call functions resulting in functions being accounted for twice. There must

diff --git a/xls/dslx/ir_convert/extract_conversion_order.h b/xls/dslx/ir_convert/extract_conversion_order.h
@@ -184,11 +184,8 @@ class ConversionRecord {
 // Args:
 //  module: Module to convert the (non-parametric) functions for.
 //  type_info: Mapping from node to type.
-//  traverse_tests: Whether to traverse DSLX test constructs. This flag should
-//    be set if we intend to run functions only called from test constructs
-//    through the JIT.
 absl::StatusOr<std::vector<ConversionRecord>> GetOrder(
-    Module* module, TypeInfo* type_info, bool traverse_tests = false);
+    Module* module, TypeInfo* type_info);
 
 // Returns a reverse topological order for functions to be converted to IR given
 // "f" as the entry function.

diff --git a/xls/dslx/ir_convert/ir_converter.cc b/xls/dslx/ir_convert/ir_converter.cc
@@ -310,11 +310,11 @@ absl::Status ConvertCallGraph(absl::Span<const ConversionRecord> order,
 
 absl::Status ConvertModuleIntoPackage(Module* module, ImportData* import_data,
                                       const ConvertOptions& options,
-                                      bool traverse_tests, Package* package) {
+                                      Package* package) {
   XLS_ASSIGN_OR_RETURN(TypeInfo * root_type_info,
                        import_data->GetRootTypeInfo(module));
   XLS_ASSIGN_OR_RETURN(std::vector<ConversionRecord> order,
-                       GetOrder(module, root_type_info, traverse_tests));
+                       GetOrder(module, root_type_info));
   PackageData package_data{package};
   XLS_RETURN_IF_ERROR(
       ConvertCallGraph(order, import_data, options, package_data));
@@ -326,11 +326,10 @@ absl::Status ConvertModuleIntoPackage(Module* module, ImportData* import_data,
 }
 
 absl::StatusOr<std::unique_ptr<Package>> ConvertModuleToPackage(
-    Module* module, ImportData* import_data, const ConvertOptions& options,
-    bool traverse_tests) {
+    Module* module, ImportData* import_data, const ConvertOptions& options) {
   auto package = std::make_unique<Package>(module->name());
   XLS_RETURN_IF_ERROR(ConvertModuleIntoPackage(module, import_data, options,
-                                               traverse_tests, package.get()));
+                                               package.get()));
   return package;
 }
 
@@ -448,7 +447,7 @@ absl::Status AddContentsToPackage(
   } else {
     XLS_RETURN_IF_ERROR(
         ConvertModuleIntoPackage(module.get(), import_data, convert_options,
-                                 /*traverse_tests=*/false, package));
+                                 package));
   }
   return absl::OkStatus();
 }

diff --git a/xls/dslx/ir_convert/ir_converter.h b/xls/dslx/ir_convert/ir_converter.h
@@ -37,23 +37,19 @@ namespace xls::dslx {
 // Args:
 //   module: Module to convert.
 //   import_data: Contains type information used in conversion.
-//   traverse_tests: Whether to convert functions called in DSLX test
-//   constructs.
-//     Note that this does NOT convert the test constructs themselves.
 //
 // Returns:
 //   The IR package that corresponds to this module.
 absl::StatusOr<std::unique_ptr<Package>> ConvertModuleToPackage(
-    Module* module, ImportData* import_data, const ConvertOptions& options,
-    bool traverse_tests = false);
+    Module* module, ImportData* import_data, const ConvertOptions& options);
 
 // As above, but the package is provided explicitly (instead of being created).
 //
 // Package must outlive this function call -- functions from "module" are placed
 // inside of it, it may not be nullptr.
 absl::Status ConvertModuleIntoPackage(Module* module, ImportData* import_data,
                                       const ConvertOptions& options,
-                                      bool traverse_tests, Package* package);
+                                      Package* package);
 
 // Wrapper around ConvertModuleToPackage that converts to IR text.
 absl::StatusOr<std::string> ConvertModule(Module* module,

diff --git a/xls/dslx/run_routines.cc b/xls/dslx/run_routines.cc
@@ -318,8 +318,7 @@ absl::StatusOr<TestResult> ParseAndTest(std::string_view program,
   if (options.run_comparator != nullptr) {
     absl::StatusOr<std::unique_ptr<Package>> ir_package_or =
         ConvertModuleToPackage(entry_module, &import_data,
-                               options.convert_options,
-                               /*traverse_tests=*/true);
+                               options.convert_options);
     if (!ir_package_or.ok()) {
       if (TryPrintError(ir_package_or.status())) {
         return TestResult::kSomeFailed;

diff --git a/xls/dslx/tests/BUILD b/xls/dslx/tests/BUILD
@@ -237,17 +237,11 @@ dslx_lang_test(name = "array_update")
 xls_dslx_test(
     name = "parametric_array_of_tuples_dslx_test",
     srcs = ["parametric_array_of_tuples.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 xls_dslx_test(
     name = "parametric_functions_dslx_test",
     srcs = ["parametric_functions.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 dslx_lang_test(
@@ -268,9 +262,6 @@ dslx_lang_test(
 xls_dslx_test(
     name = "bit_slice_dslx_test",
     srcs = ["bit_slice.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 dslx_lang_test(name = "bit_slice_update")
@@ -281,18 +272,12 @@ dslx_lang_test(name = "const_slice_limit")
 xls_dslx_test(
     name = "bit_slice_syntax_dslx_test",
     srcs = ["bit_slice_syntax.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 # Note: no meaningful function to convert.
 xls_dslx_test(
     name = "tuple_indexing_dslx_test",
     srcs = ["tuple_indexing.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 dslx_lang_test(name = "clz")
@@ -303,18 +288,12 @@ dslx_lang_test(name = "ctz")
 xls_dslx_test(
     name = "one_hot_dslx_test",
     srcs = ["one_hot.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 # Note: no meaningful function to convert.
 xls_dslx_test(
     name = "one_hot_sel",
     srcs = ["one_hot_sel.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 # Note: no meaningful function to convert.
@@ -331,18 +310,12 @@ dslx_lang_test(name = "signed_number_type")
 xls_dslx_test(
     name = "numerical_conversions_dslx_test",
     srcs = ["numerical_conversions.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 # Note: no meaningful function to convert.
 xls_dslx_test(
     name = "numerical_conversions2_dslx_test",
     srcs = ["numerical_conversions2.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 dslx_lang_test(
@@ -359,44 +332,29 @@ xls_dslx_test(
 xls_dslx_test(
     name = "enum_values_dslx_test",
     srcs = ["enum_values.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 # TODO(leary); 2019-10-24 Enable when we have ConcatArray op.
 xls_dslx_test(
     name = "casts_dslx_test",
     srcs = ["casts.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 # TODO(leary); 2019-10-24 Enable when we have ConcatArray op.
 xls_dslx_test(
     name = "cast_to_array_dslx_test",
     srcs = ["cast_to_array.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 xls_dslx_test(
     name = "parametric_with_comparison_dslx_test",
     srcs = ["parametric_with_comparison.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 # Note: no meaningful function to convert.
 xls_dslx_test(
     name = "parametric_smul_dslx_test",
     srcs = ["parametric_smul.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 # Note: no meaningful function to convert.
@@ -415,9 +373,6 @@ dslx_lang_test(name = "basic_struct_attr")
 xls_dslx_test(
     name = "struct_equality_dslx_test",
     srcs = ["struct_equality.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 dslx_lang_test(name = "basic_struct_update")
@@ -813,9 +768,6 @@ dslx_lang_test(name = "for_up_to_constexpr_cast")
 xls_dslx_test(
     name = "bits_concat_dslx_test",
     srcs = ["bits_concat.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
 )
 
 dslx_lang_test(name = "local_const_value_in_parametric_match")

diff --git a/xls/examples/BUILD b/xls/examples/BUILD
@@ -163,9 +163,6 @@ xls_dslx_library(
 xls_dslx_test(
     name = "fir_filter_dslx_test",
     srcs = ["fir_filter.x"],
-    dslx_test_args = {
-        "compare": "none",
-    },
     deps = [":fir_filter_dslx"],
 )
 
@@ -174,9 +171,6 @@ xls_dslx_test(
     srcs = ["matmul_4x4.x"],
     # TODO(rspringer): 2022-05-24: Does not successfully lower to IR.
     # Needs support for arrays of channels.
-    dslx_test_args = {
-        "compare": "none",
-    },
     deps = [
         "//xls/modules/fp:fp32_mul_2",
     ],
@@ -454,9 +448,6 @@ xls_dslx_library(
 
 xls_dslx_test(
     name = "ram_test",
-    dslx_test_args = {
-        "compare": "none",
-    },
     library = ":ram_dslx",
 )
 
@@ -468,9 +459,6 @@ xls_dslx_library(
 
 xls_dslx_test(
     name = "delay_test",
-    dslx_test_args = {
-        "compare": "none",
-    },
     library = ":delay_dslx",
 )
 

diff --git a/xls/flows/ir_wrapper.cc b/xls/flows/ir_wrapper.cc
@@ -182,7 +182,7 @@ absl::StatusOr<IrWrapper> IrWrapper::Create(
 
   XLS_RET_CHECK_OK(dslx::ConvertModuleIntoPackage(
       ir_wrapper.top_module_, &ir_wrapper.import_data_, convert_options,
-      /*traverse_tests=*/false, ir_wrapper.package_.get()));
+      ir_wrapper.package_.get()));
 
   XLS_VLOG(3) << "IrWrapper Package (pre-opt):";
   XLS_VLOG_LINES(3, ir_wrapper.package_->DumpIr());

diff --git a/xls/tools/repl.cc b/xls/tools/repl.cc
@@ -275,8 +275,7 @@ absl::Status UpdateIr() {
   XLS_ASSIGN_OR_RETURN(globals->ir_package,
                        ConvertModuleToPackage(globals->dslx->module.get(),
                                               &globals->dslx->import_data,
-                                              dslx::ConvertOptions{},
-                                              /*traverse_tests=*/true));
+                                              dslx::ConvertOptions{}));
   XLS_ASSIGN_OR_RETURN(std::string mangled_name,
                        dslx::MangleDslxName(globals->dslx->module.get()->name(),
                                             "main", /*free_keys=*/{}));