diff --git a/xls/dslx/frontend/BUILD b/xls/dslx/frontend/BUILD index dc0acc7729..5128949d19 100644 --- a/xls/dslx/frontend/BUILD +++ b/xls/dslx/frontend/BUILD @@ -834,6 +834,22 @@ cc_library( ], ) +cc_library( + name = "test_function_transformer", + srcs = ["test_function_transformer.cc"], + hdrs = ["test_function_transformer.h"], + deps = [ + ":ast", + ":ast_cloner", + ":ast_node_visitor_with_default", + ":module", + "//xls/common:attribute_data", + "//xls/common/status:status_macros", + "//xls/dslx:import_data", + "@abseil-cpp//absl/status", + ], +) + cc_test( name = "fuzz_domain_rewriter_test", srcs = ["fuzz_domain_rewriter_test.cc"], diff --git a/xls/dslx/frontend/test_function_transformer.cc b/xls/dslx/frontend/test_function_transformer.cc new file mode 100644 index 0000000000..7bbe182629 --- /dev/null +++ b/xls/dslx/frontend/test_function_transformer.cc @@ -0,0 +1,113 @@ +// Copyright 2026 The XLS Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "xls/dslx/frontend/test_function_transformer.h" + +#include +#include +#include + +#include "absl/status/status.h" +#include "xls/common/attribute_data.h" +#include "xls/common/status/status_macros.h" +#include "xls/dslx/frontend/ast.h" +#include "xls/dslx/frontend/ast_cloner.h" +#include "xls/dslx/frontend/ast_node_visitor_with_default.h" +#include "xls/dslx/frontend/module.h" + +namespace xls { +namespace dslx { + +namespace { +class SpawnFinder : public AstNodeVisitorWithDefault { + public: + // Legacy spawn statement, not the new spawn attribute. + absl::Status HandleSpawn(const Spawn* node) override { + has_spawn_ = true; + return DefaultHandler(node); + } + + absl::Status HandleInvocation(const Invocation* node) override { + // Trait-based spawn + if (const Attr* attr = dynamic_cast(node->callee()); + attr != nullptr) { + has_spawn_ = attr->attr() == "spawn"; + } + return absl::OkStatus(); + } + + absl::Status HandleFunction(const Function* node) override { + has_spawn_ = false; + if (GetAttribute(node, AttributeKind::kTest).has_value()) { + // Skip functions that aren't test functions. + return absl::OkStatus(); + } + // Recurse into children to see if there are any spawns. + return DefaultHandler(node); + } + absl::Status DefaultHandler(const AstNode* node) override { + for (const auto& child : node->GetChildren(false)) { + XLS_RETURN_IF_ERROR(child->Accept(this)); + } + return absl::OkStatus(); + } + + bool has_spawn() const { return has_spawn_; } + + private: + bool has_spawn_ = false; +}; + +absl::StatusOr TransmuteToTestProc(const Function* fn, + Module* module) { + // TODO(davidplass): Implement this. + return nullptr; +} + +} // namespace + +absl::StatusOr> TransformTestFunctions( + const Module& module) { + // Cannot reserve space in the vector because we don't know how many functions + // will be transformed. + std::vector test_functions_with_spawn; + + // 1. Find all test functions with spawns. + for (auto* func : module.GetFunctions()) { + SpawnFinder spawn_finder; + XLS_RETURN_IF_ERROR(func->Accept(&spawn_finder)); + if (spawn_finder.has_spawn()) { + test_functions_with_spawn.push_back(func); + } + } + + // 2. Clone the module, removing test functions with spawns. + XLS_ASSIGN_OR_RETURN( + std::unique_ptr cloned_module, + CloneModuleRemovingMembers(module, test_functions_with_spawn)); + + // 3. For each test function with a spawn, transform it into a TestProc and + // add to the cloned module. + for (auto* node : test_functions_with_spawn) { + const Function* func = dynamic_cast(node); + XLS_ASSIGN_OR_RETURN(TestProc * test_proc, + TransmuteToTestProc(func, cloned_module.get())); + // TODO(davidplass): Add collision error handler using the node's location + XLS_RETURN_IF_ERROR( + cloned_module->AddTop(test_proc, /*make_collision_error=*/nullptr)); + } + return cloned_module; +} +} // namespace dslx +} // namespace xls diff --git a/xls/dslx/frontend/test_function_transformer.h b/xls/dslx/frontend/test_function_transformer.h new file mode 100644 index 0000000000..90edf24a53 --- /dev/null +++ b/xls/dslx/frontend/test_function_transformer.h @@ -0,0 +1,34 @@ +// Copyright 2026 The XLS Authors +// +// 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. + +#ifndef XLS_DSLX_FRONTEND_TEST_FUNCTION_TRANSFORMER_H_ +#define XLS_DSLX_FRONTEND_TEST_FUNCTION_TRANSFORMER_H_ + +#include + +#include "absl/status/status.h" +#include "xls/dslx/frontend/module.h" + +namespace xls { +namespace dslx { + +// Transforms test functions that spawn procs into TestProcs in the cloned +// module. The original functions will be removed from the cloned module. +absl::StatusOr> TransformTestFunctions( + const Module& module); + +} // namespace dslx +} // namespace xls + +#endif // XLS_DSLX_FRONTEND_TEST_FUNCTION_TRANSFORMER_H_ diff --git a/xls/dslx/ir_convert/ir_converter_legacy_test.cc b/xls/dslx/ir_convert/ir_converter_legacy_test.cc index 22b5fbda16..6d9e25b2fd 100644 --- a/xls/dslx/ir_convert/ir_converter_legacy_test.cc +++ b/xls/dslx/ir_convert/ir_converter_legacy_test.cc @@ -3205,7 +3205,7 @@ TEST_F(IrConverterLegacyTest, InvalidChannelDecl) { auto import_data = CreateImportDataForTest(); EXPECT_THAT( ConvertOneFunctionForTest(program, "main", import_data, kNoPosOptions), - StatusIs(absl::StatusCode::kInternal, + StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr("Channels can only be declared in"))); } @@ -3225,8 +3225,8 @@ fn main() { auto import_data = CreateImportDataForTest(); EXPECT_THAT( ConvertOneFunctionForTest(program, "main", import_data, kNoPosOptions), - StatusIs(absl::StatusCode::kUnimplemented, - HasSubstr("Functions cannot spawn procs."))); + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Cannot spawn outside"))); } TEST_F(IrConverterLegacyTest, InvalidSpawnProcScopedChannel) { @@ -3244,8 +3244,8 @@ fn main() { auto import_data = CreateImportDataForTest(); EXPECT_THAT(ConvertOneFunctionForTest(program, "main", import_data, kProcScopedChannelOptions), - StatusIs(absl::StatusCode::kUnimplemented, - HasSubstr("Functions cannot spawn procs."))); + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Cannot spawn outside"))); } TEST_F(IrConverterLegacyTest, InvalidSpawnInNextProcScoped) { diff --git a/xls/dslx/ir_convert/ir_converter_test.cc b/xls/dslx/ir_convert/ir_converter_test.cc index 50e5f821d2..24c82c21ef 100644 --- a/xls/dslx/ir_convert/ir_converter_test.cc +++ b/xls/dslx/ir_convert/ir_converter_test.cc @@ -3232,8 +3232,9 @@ TEST_F(IrConverterTest, InvalidChannelDecl) { })"; auto import_data = CreateImportDataForTest(); + // This is now a typecheck violation instead of IR Converter check. EXPECT_THAT(ConvertOneFunctionForTest(program, "main", import_data), - StatusIs(absl::StatusCode::kInternal, + StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr("Channels can only be declared in"))); } @@ -3252,8 +3253,8 @@ fn main() { auto import_data = CreateImportDataForTest(); EXPECT_THAT(ConvertOneFunctionForTest(program, "main", import_data), - StatusIs(absl::StatusCode::kUnimplemented, - HasSubstr("Functions cannot spawn procs."))); + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Cannot spawn outside a"))); } TEST_F(IrConverterTest, InvalidSpawnProcScopedChannel) { @@ -3270,8 +3271,8 @@ fn main() { )"; auto import_data = CreateImportDataForTest(); EXPECT_THAT(ConvertOneFunctionForTest(program, "main", import_data), - StatusIs(absl::StatusCode::kUnimplemented, - HasSubstr("Functions cannot spawn procs."))); + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("Cannot spawn outside"))); } TEST_F(IrConverterTest, InvalidSpawnInNextProcScoped) { diff --git a/xls/dslx/type_system_v2/BUILD b/xls/dslx/type_system_v2/BUILD index 99e469768d..6401256e1c 100644 --- a/xls/dslx/type_system_v2/BUILD +++ b/xls/dslx/type_system_v2/BUILD @@ -223,6 +223,7 @@ cc_library( ":type_system_tracer", ":unify_type_annotations", ":validate_concrete_type", + "//xls/common:attribute_data", "//xls/common/status:ret_check", "//xls/common/status:status_macros", "//xls/dslx:constexpr_evaluator", diff --git a/xls/dslx/type_system_v2/constant_collector.cc b/xls/dslx/type_system_v2/constant_collector.cc index c813e66608..bec9a3a1de 100644 --- a/xls/dslx/type_system_v2/constant_collector.cc +++ b/xls/dslx/type_system_v2/constant_collector.cc @@ -805,9 +805,10 @@ class Visitor : public AstNodeVisitorWithDefault { XLS_ASSIGN_OR_RETURN( std::optional proc_def, GetContainingStructOrProcDef(invocation, import_data_)); - XLS_RET_CHECK(proc_def.has_value()); - ti_->AddProcDefSpawn(absl::down_cast(*proc_def), - external_initializer); + if (proc_def.has_value()) { + ti_->AddProcDefSpawn(absl::down_cast(*proc_def), + external_initializer); + } return absl::OkStatus(); } diff --git a/xls/dslx/type_system_v2/inference_table_converter_impl.cc b/xls/dslx/type_system_v2/inference_table_converter_impl.cc index 0294b521ca..7f382189ac 100644 --- a/xls/dslx/type_system_v2/inference_table_converter_impl.cc +++ b/xls/dslx/type_system_v2/inference_table_converter_impl.cc @@ -42,6 +42,7 @@ #include "absl/strings/str_join.h" #include "absl/strings/substitute.h" #include "absl/types/span.h" +#include "xls/common/attribute_data.h" #include "xls/common/status/ret_check.h" #include "xls/common/status/status_macros.h" #include "xls/dslx/constexpr_evaluator.h" @@ -534,6 +535,39 @@ class InferenceTableConverterImpl : public InferenceTableConverter, file_table_); } + // Only allow calling (new-style) spawn from inside a proc (legacy or + // impl-based), or (new!) a test function. + XLS_ASSIGN_OR_RETURN(bool is_proc_def_spawn, + IsProcDefSpawnFunction(function)); + if (is_proc_def_spawn) { + // If the spawn is in a proc, OK. + // If the spawn is in a test function, OK. + // Otherwise, error. + bool valid_spawn = false; + if (caller.has_value()) { + // Legacy procs can call new-style spawn. + valid_spawn = (*caller)->IsInProc(); + + XLS_ASSIGN_OR_RETURN( + std::optional containing_struct_or_proc, + GetContainingStructOrProcDef(invocation, import_data_)); + if (containing_struct_or_proc.has_value() && + (*containing_struct_or_proc)->kind() == AstNodeKind::kProcDef) { + // impl-based procs can call spawn + valid_spawn = true; + } else { + // Test functions can call spawn + valid_spawn |= + GetAttribute(*caller, AttributeKind::kTest).has_value(); + } + } + if (!valid_spawn) { + return TypeInferenceErrorStatus(invocation->span(), nullptr, + "Cannot spawn outside a proc or test.", + file_table_); + } + } + // Come up with the actual args by merging the possible target object // (`some_struct` in the case of `some_struct.foo(args)`), with the vector // of explicit actual args. diff --git a/xls/dslx/type_system_v2/populate_table_visitor.cc b/xls/dslx/type_system_v2/populate_table_visitor.cc index 532f43defc..1b02cde239 100644 --- a/xls/dslx/type_system_v2/populate_table_visitor.cc +++ b/xls/dslx/type_system_v2/populate_table_visitor.cc @@ -170,6 +170,11 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, absl::Status HandleChannelDecl(const ChannelDecl* node) override { VLOG(5) << "HandleChannelDecl: " << node->ToString() << " with type: " << node->type()->ToString(); + if (!handle_proc_functions_ && !in_test_fn_ && !in_parametric_fn_) { + return TypeInferenceErrorStatus( + node->span(), /*type=*/nullptr, + "Channels can only be declared in a proc or test.", file_table_); + } if (node->fifo_depth().has_value()) { Expr* fifo_depth = *node->fifo_depth(); XLS_RETURN_IF_ERROR(DefineAndSetTypeVariable(fifo_depth, "fifo_depth")); @@ -1422,6 +1427,14 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, absl::Status HandleTrait(const Trait*) override { return absl::OkStatus(); } + absl::Status HandleTestFunction(const TestFunction* node) override { + VLOG(5) << "HandleTestFunction: " << node->ToString(); + in_test_fn_ = true; + XLS_RETURN_IF_ERROR(DefaultHandler(node)); + in_test_fn_ = false; + return absl::OkStatus(); + } + absl::Status HandleFunction(const Function* node) override { // Proc functions are reachable via both the `Module` and the `Proc`, as an // oddity of how procs are set up in the AST. We only want to handle them in @@ -1430,6 +1443,7 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, if (node->IsInProc() && !handle_proc_functions_) { return absl::OkStatus(); } + in_parametric_fn_ = node->IsParametric(); VLOG(5) << "HandleFunction: " << node->ToString() << ", parametric: " << node->IsParametric(); @@ -1437,6 +1451,9 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, XLS_RETURN_IF_ERROR(binding->Accept(this)); } + bool was_in_test_fn = in_test_fn_; + in_test_fn_ |= GetAttribute(node, AttributeKind::kTest).has_value(); + const TypeAnnotation* return_type = GetReturnType(module_, *node); XLS_RETURN_IF_ERROR(return_type->Accept(this)); for (const Param* param : node->params()) { @@ -1471,7 +1488,10 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, } // Descend into the function body. - return node->body()->Accept(this); + auto body_status = node->body()->Accept(this); + in_test_fn_ = was_in_test_fn; + in_parametric_fn_ = false; + return body_status; } absl::Status HandleFuzzTestFunction(const FuzzTestFunction* node) override { @@ -1551,6 +1571,12 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, absl::Status HandleSpawn(const Spawn* node) override { VLOG(5) << "HandleSpawn: " << node->ToString(); + if (!handle_proc_functions_ && !in_test_fn_) { + return TypeInferenceErrorStatus( + *node->GetSpan(), nullptr, + absl::Substitute("Cannot spawn outside a `proc` or proc test"), + file_table_); + } XLS_RETURN_IF_ERROR(DefineAndSetTypeVariable(node->config(), "config")); XLS_RETURN_IF_ERROR(DefineAndSetTypeVariable(node->next(), "next")); XLS_RETURN_IF_ERROR(table_.SetTypeAnnotation( @@ -1607,11 +1633,13 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, VLOG(5) << "HandleInvocation: " << node->ToString(); - // If we're outside a proc, we can't call proc-only builtins. - if (!handle_proc_functions_ && ProcOnlyFunction(node->callee())) { + // If we're outside a proc, we can't call proc-only builtins, except if + // we're in a test function. + if (!(handle_proc_functions_ || in_test_fn_) && + ProcOnlyFunction(node->callee())) { return TypeInferenceErrorStatus( *node->GetSpan(), nullptr, - absl::Substitute("Cannot call `$0` outside a `proc`", + absl::Substitute("Cannot call `$0` outside a `proc` or proc test", node->callee()->ToString()), file_table_); } @@ -2249,6 +2277,8 @@ class PopulateInferenceTableVisitor : public PopulateTableVisitor, TypecheckModuleFn typecheck_imported_module_; bool handle_proc_functions_ = false; bool in_fuzz_test_domain_ = false; + bool in_test_fn_ = false; + bool in_parametric_fn_ = false; }; } // namespace diff --git a/xls/dslx/type_system_v2/typecheck_module_v2_proc_test.cc b/xls/dslx/type_system_v2/typecheck_module_v2_proc_test.cc index 3d52a31cc9..fb0e456336 100644 --- a/xls/dslx/type_system_v2/typecheck_module_v2_proc_test.cc +++ b/xls/dslx/type_system_v2/typecheck_module_v2_proc_test.cc @@ -719,6 +719,201 @@ proc Foo { "Use of `Self` inside legacy procs is not supported."))); } +TEST(TypecheckV2ProcTest, SpawnProcWithImplInTestFunctionSucceeds) { + EXPECT_THAT(R"( +proc PassThrough { + c_in: chan in, + c_out: chan out, +} + +impl PassThrough { + fn new(c_in: chan in, c_out: chan out) -> Self { + PassThrough { c_in, c_out } + } + fn next(self) { + let (tok, val) = recv(join(), self.c_in); + send(tok, self.c_out, val); + } +} + +#[test] +fn test_pass_through() { + let (in_w, in_r) = chan("in"); + let (out_w, out_r) = chan("out"); + PassThrough::new(in_r, out_w).spawn(); + let tok = send(join(), in_w, u32:42); + let (tok, val) = recv(tok, out_r); + assert_eq(val, u32:42) +} +)", + TypecheckSucceeds(::testing::_)); +} + +TEST(TypecheckV2ProcTest, ChannelOpsInTestFunctionSucceeds) { + EXPECT_THAT(R"( +#[test] +fn my_test_fn() { + let (p, c) = chan("test_chan"); + let tok = send(join(), p, u32:42); + let (tok, val) = recv(tok, c); + assert_eq(val, u32:42) +} +)", + TypecheckSucceeds(::testing::_)); +} + +TEST(TypecheckV2ProcTest, ChannelDeclInNormalFunctionFails) { + EXPECT_THAT(R"( +fn bad_function() { + let (p, c) = chan("test_chan"); + () +} +)", + TypecheckFails(HasSubstr( + "Channels can only be declared in a proc or test"))); +} + +TEST(TypecheckV2ProcTest, ChannelSendInNormalFunctionFails) { + EXPECT_THAT(R"( +fn bad_function(p: chan out) { + let tok = send(join(), p, u32:42); + () +} +)", + TypecheckFails(HasSubstr( + "Cannot call `send` outside a `proc` or proc test"))); +} + +TEST(TypecheckV2ProcTest, NewSpawnInNormalFunctionFails) { + EXPECT_THAT( + R"( +proc Dummy { + c: chan out, +} +impl Dummy { + fn new(c: chan out) -> Self { + Dummy { c } + } + fn next(self) {} +} + +fn bad_function(p: chan out) { + Dummy::new(p).spawn(); + () +} +)", + TypecheckFails(HasSubstr("Cannot spawn outside a proc or test."))); +} + +TEST(TypecheckV2ProcTest, ChannelOpsInTestUtilityFails) { + EXPECT_THAT(R"( +#[cfg(test)] +fn helper(p: chan out, c: chan in) -> u32 { + let tok = send(join(), p, u32:42); + let (tok, val) = recv(tok, c); + val +} +)", + TypecheckFails(HasSubstr( + "Cannot call `send` outside a `proc` or proc test"))); +} + +TEST(TypecheckV2ProcTest, SpawnInTestUtilityFails) { + EXPECT_THAT( + R"( +proc Dummy { + c: chan out, +} +impl Dummy { + fn new(c: chan out) -> Self { + Dummy { c } + } + fn next(self) {} +} + +#[cfg(test)] +fn helper(p: chan out) { + Dummy::new(p).spawn(); + () +} +)", + TypecheckFails(HasSubstr("Cannot spawn outside a proc or test."))); +} + +TEST(TypecheckV2ProcTest, ChannelRecvInNormalFunctionFails) { + EXPECT_THAT(R"( +fn bad_function(c: chan in) { + let (tok, val) = recv(join(), c); + () +} +)", + TypecheckFails(HasSubstr( + "Cannot call `recv` outside a `proc` or proc test"))); +} + +TEST(TypecheckV2ProcTest, ChannelJoinInNormalFunctionFails) { + EXPECT_THAT(R"( +fn bad_function() { + let tok = join(); + () +} +)", + TypecheckFails(HasSubstr( + "Cannot call `join` outside a `proc` or proc test"))); +} + +TEST(TypecheckV2ProcTest, ChannelRecvInTestUtilityFails) { + EXPECT_THAT(R"( +#[cfg(test)] +fn helper(c: chan in) -> u32 { + let (tok, val) = recv(join(), c); + val +} +)", + TypecheckFails(HasSubstr( + "Cannot call `recv` outside a `proc` or proc test"))); +} + +TEST(TypecheckV2ProcTest, ChannelJoinInTestUtilityFails) { + EXPECT_THAT(R"( +#[cfg(test)] +fn helper() -> token { + join() +} +)", + TypecheckFails(HasSubstr( + "Cannot call `join` outside a `proc` or proc test"))); +} + +TEST(TypecheckV2ProcTest, SpawnImportedProcDefInTestFunctionSucceeds) { + std::string_view kImported = R"( +pub proc P { + c_out: chan out, +} +impl P { + fn new(c_out: chan out) -> Self { + P { c_out: c_out } + } + fn next(self) {} +} +)"; + + constexpr std::string_view kProgram = R"( +import imported; + +#[test] +fn my_test() { + let (c_out, c_in) = chan("my_chan"); + imported::P::new(c_out).spawn(); + () +} +)"; + + ImportData import_data = CreateImportDataForTest(); + XLS_EXPECT_OK(TypecheckV2(kImported, "imported", &import_data).status()); + XLS_EXPECT_OK(TypecheckV2(kProgram, "main", &import_data)); +} + TEST(TypecheckV2ProcTest, ProcDefWithImportedStructMember) { constexpr std::string_view kImported = R"( pub struct Foo {