Refactor Verilog fuzzing to use a VerilogGenerator struct.

This change introduces a `VerilogGenerator` struct that encapsulates the XLS package, scheduling options, and codegen options. The fuzz domains now generate instances of `VerilogGenerator`, and the actual Verilog generation is performed by calling the `GenerateVerilog()` method on this struct. This allows fuzz tests to access the inputs (package and options) even when codegen fails, aiding in debugging.

PiperOrigin-RevId: 872027638

Author: allight

xls/fuzzer/verilog_fuzzer/BUILD

@@ -36,6 +36,7 @@ cc_library(
         "//xls/public:runtime_codegen_actions",
         "//xls/tools:codegen_flags_cc_proto",
         "//xls/tools:scheduling_options_flags_cc_proto",
+        "@com_google_absl//absl/log",
         "@com_google_absl//absl/log:check",
         "@com_google_absl//absl/status:statusor",
     ],
@@ -48,6 +49,7 @@ cc_test(
         ":verilog_fuzz_domain",
         "//xls/common:xls_gunit_main",
         "//xls/common/fuzzing:fuzztest",
+        "//xls/fuzzer/ir_fuzzer:ir_fuzz_domain",
         "//xls/tools:codegen_flags_cc_proto",
         "//xls/tools:scheduling_options_flags_cc_proto",
         "@com_google_absl//absl/status",

xls/fuzzer/verilog_fuzzer/codegen_fuzz_test.cc

@@ -20,6 +20,7 @@
 #include "absl/status/status.h"
 #include "absl/status/status_matchers.h"
 #include "absl/status/statusor.h"
+#include "xls/fuzzer/ir_fuzzer/ir_fuzz_domain.h"
 #include "xls/fuzzer/verilog_fuzzer/verilog_fuzz_domain.h"
 #include "xls/tools/codegen_flags.pb.h"
 #include "xls/tools/scheduling_options_flags.pb.h"
@@ -36,6 +37,13 @@ using ::testing::Not;
 
 constexpr std::string_view kTop = "dut";
 
+MATCHER_P(HasVerilogText, text,
+          absl::StrCat("Expected verilog text to match ",
+                       testing::DescribeMatcher<std::string>(text))) {
+  return testing::ExplainMatchResult(text, arg.codegen_result.verilog_text,
+                                     result_listener);
+}
+
 SchedulingOptionsFlagsProto DefaultSchedulingOptions() {
   SchedulingOptionsFlagsProto scheduling_options;
   scheduling_options.set_delay_model("sky130");
@@ -55,15 +63,18 @@ CodegenFlagsProto DefaultCodegenOptions() {
 }
 
 void CodegenSucceedsForEveryFunctionWithNoPipelineLimit(
-    absl::StatusOr<std::string> verilog) {
-  EXPECT_THAT(verilog, IsOkAndHolds(Not(IsEmpty())));
+    VerilogGenerator verilog) {
+  auto result = verilog.GenerateVerilog();
+  EXPECT_THAT(result, IsOkAndHolds(HasVerilogText(Not(IsEmpty()))))
+      << result.status();
 }
+
 FUZZ_TEST(CodegenFuzzTest, CodegenSucceedsForEveryFunctionWithNoPipelineLimit)
-    .WithDomains(StatusOrVerilogFuzzDomain(DefaultSchedulingOptions(),
-                                           DefaultCodegenOptions()));
+    .WithDomains(VerilogGeneratorDomain(
+        IrFuzzDomain(), fuzztest::Just(DefaultSchedulingOptions()),
+        fuzztest::Just(DefaultCodegenOptions())));
 
-void CodegenSucceedsOrThrowsReasonableError(
-    absl::StatusOr<std::string> verilog) {
+void CodegenSucceedsOrThrowsReasonableError(VerilogGenerator verilog) {
   // We check for success or expected errors here. The expected errors are:
   // - absl::NotFoundError("No delay estimator found named")- an
   //   incorrectly-named delay estimator in the scheduling options.
@@ -77,8 +88,8 @@ void CodegenSucceedsOrThrowsReasonableError(
   // We don't expect:
   // - CHECK-fails, OOMs, etc.
   // - Random absl::InternalError(), or other "unexpected" errors.
-  EXPECT_THAT(verilog,
-              AnyOf(IsOkAndHolds(Not(IsEmpty())),
+  EXPECT_THAT(verilog.GenerateVerilog(),
+              AnyOf(IsOkAndHolds(HasVerilogText(Not(IsEmpty()))),
                     StatusIs(absl::StatusCode::kNotFound,
                              HasSubstr("No delay estimator found named")),
                     StatusIs(absl::StatusCode::kInternal,
@@ -89,9 +100,9 @@ void CodegenSucceedsOrThrowsReasonableError(
                                    absl::StatusCode::kUnimplemented))));
 }
 FUZZ_TEST(CodegenFuzzTest, CodegenSucceedsOrThrowsReasonableError)
-    .WithDomains(fuzztest::FlatMap(StatusOrVerilogFuzzDomain,
-                                   NoFdoSchedulingOptionsFlagsDomain(),
-                                   CodegenFlagsDomain()));
+    .WithDomains(VerilogGeneratorDomain(IrFuzzDomain(),
+                                        NoFdoSchedulingOptionsFlagsDomain(),
+                                        CodegenFlagsDomain()));
 
 }  // namespace
 }  // namespace xls

xls/fuzzer/verilog_fuzzer/verilog_fuzz_domain.cc

@@ -16,9 +16,11 @@
 
 #include <memory>
 #include <string>
+#include <utility>
 
 #include "xls/common/fuzzing/fuzztest.h"
 #include "absl/log/check.h"
+#include "absl/log/log.h"
 #include "absl/status/statusor.h"
 #include "xls/common/logging/log_lines.h"
 #include "xls/common/status/ret_check.h"
@@ -31,51 +33,65 @@
 
 namespace xls {
 
-fuzztest::Domain<absl::StatusOr<std::string>> StatusOrVerilogFuzzDomain(
-    SchedulingOptionsFlagsProto scheduling_options,
-    CodegenFlagsProto codegen_options) {
-  return fuzztest::Map(
-      [scheduling_options, codegen_options](
-          std::shared_ptr<Package> package) -> absl::StatusOr<std::string> {
-        XLS_RET_CHECK_GE(package->functions().size(), 1)
-            << "We expect IrFuzzDomain() to return a package with at least one "
-               "function.\n"
-            << package->DumpIr();
-        // Make the function the top-level module.
-        XLS_RETURN_IF_ERROR(package->SetTop(package->functions().front().get()))
-            << "Failed to set top-level module in:\n"
-            << package->DumpIr();
+fuzztest::Domain<VerilogGenerator> VerilogGeneratorDomain(
+    fuzztest::Domain<std::shared_ptr<Package>>&& package_domain,
+    fuzztest::Domain<SchedulingOptionsFlagsProto>&& scheduling_options,
+    fuzztest::Domain<CodegenFlagsProto>&& codegen_options) {
+  return fuzztest::StructOf<VerilogGenerator>(std::move(package_domain),
+                                              std::move(scheduling_options),
+                                              std::move(codegen_options));
+}
 
-        XLS_ASSIGN_OR_RETURN(
-            (auto [scheduling_result, codegen_result]),
-            ScheduleAndCodegenPackage(package.get(), scheduling_options,
-                                      codegen_options,
-                                      /*with_delay_model=*/true),
-            _ << " while scheduling and codegening package:\n"
-              << package->DumpIr());
+absl::StatusOr<ScheduleAndCodegenResult> VerilogGenerator::GenerateVerilog() {
+  XLS_RET_CHECK_GE(package->functions().size(), 1)
+      << "We expect IrFuzzDomain() to return a package with at least one "
+         "function.\n"
+      << package->DumpIr();
+  // Make the function the top-level module.
+  XLS_RETURN_IF_ERROR(package->SetTop(package->functions().front().get()))
+      << "Failed to set top-level module in:\n"
+      << package->DumpIr();
 
-        XLS_LOG_LINES(INFO, package->DumpIr());
-        XLS_LOG_LINES(INFO, codegen_result.verilog_text);
+  XLS_ASSIGN_OR_RETURN(auto result,
+                       ScheduleAndCodegenPackage(
+                           package.get(), scheduling_options, codegen_options,
+                           /*with_delay_model=*/true),
+                       _ << " while scheduling and codegening package:\n"
+                         << package->DumpIr());
 
-        return codegen_result.verilog_text;
-      },
-      IrFuzzDomain());
+  if (VLOG_IS_ON(2)) {
+    XLS_LOG_LINES(INFO, package->DumpIr());
+    XLS_LOG_LINES(INFO, result.codegen_result.verilog_text);
+  }
+
+  return std::move(result);
+}
+
+namespace internal {
+std::string UnwrapStatusOrVerilog(
+    const absl::StatusOr<ScheduleAndCodegenResult>& verilog) {
+  CHECK_OK(verilog.status());
+  return verilog->codegen_result.verilog_text;
+}
+absl::StatusOr<ScheduleAndCodegenResult> DoGenerateVerilog(
+    VerilogGenerator verilog) {
+  return verilog.GenerateVerilog();
 }
+}  // namespace internal
 
 fuzztest::Domain<std::string> VerilogFuzzDomain(
     SchedulingOptionsFlagsProto scheduling_options,
     CodegenFlagsProto codegen_options) {
   return fuzztest::Map(
-      [](absl::StatusOr<std::string> verilog) -> std::string {
-        // Should be filtered
-        CHECK_OK(verilog.status());
-        return *verilog;
-      },
+      &internal::UnwrapStatusOrVerilog,
       fuzztest::Filter(
-          [](const absl::StatusOr<std::string> &verilog) {
+          [](const absl::StatusOr<ScheduleAndCodegenResult>& verilog) {
             return verilog.ok();
           },
-          StatusOrVerilogFuzzDomain(scheduling_options, codegen_options)));
+          fuzztest::Map(internal::DoGenerateVerilog,
+                        VerilogGeneratorDomain(
+                            IrFuzzDomain(), fuzztest::Just(scheduling_options),
+                            fuzztest::Just(codegen_options)))));
 }
 
 fuzztest::Domain<CodegenFlagsProto> CodegenFlagsDomain() {

xls/fuzzer/verilog_fuzzer/verilog_fuzz_domain.h

@@ -19,19 +19,21 @@
 
 #include "xls/common/fuzzing/fuzztest.h"
 #include "absl/status/statusor.h"
+#include "xls/fuzzer/ir_fuzzer/ir_fuzz_domain.h"
+#include "xls/ir/package.h"
+#include "xls/public/runtime_codegen_actions.h"
 #include "xls/tools/codegen_flags.pb.h"
 #include "xls/tools/scheduling_options_flags.pb.h"
 
 namespace xls {
 
-// A domain that generates standalone Verilog files.
-// These files are created by taking the output of IrFuzzDomain() and
-// running codegen on it with the provided options. The returned domain
-// contains a StatusOr<> because codegen can fail for a variety of reasons,
-// e.g. scheduling options that cannot be satisfied.
-fuzztest::Domain<absl::StatusOr<std::string>> StatusOrVerilogFuzzDomain(
-    SchedulingOptionsFlagsProto scheduling_options,
-    CodegenFlagsProto codegen_options);
+struct VerilogGenerator {
+  std::shared_ptr<Package> package;
+  SchedulingOptionsFlagsProto scheduling_options;
+  CodegenFlagsProto codegen_options;
+
+  absl::StatusOr<ScheduleAndCodegenResult> GenerateVerilog();
+};
 
 // A domain like StatusOrVerilogFuzzDomain that filters out any inputs that are
 // infeasible given the provided scheduling and codegen options.
@@ -48,6 +50,26 @@ fuzztest::Domain<CodegenFlagsProto> CodegenFlagsDomain();
 fuzztest::Domain<SchedulingOptionsFlagsProto>
 NoFdoSchedulingOptionsFlagsDomain();
 
+// A domain that can be used to create verilog out of arbitrary xls packages.
+//
+// To get the actual verilog the user should call the 'GenerateVerilog()'
+// function. This domain may not always be able to generate verilog because
+// some combinations of XLS packages and scheduling/codegen options are
+// infeasible.
+fuzztest::Domain<VerilogGenerator> VerilogGeneratorDomain(
+    fuzztest::Domain<std::shared_ptr<Package>>&& package_domain =
+        IrFuzzDomain(),
+    fuzztest::Domain<SchedulingOptionsFlagsProto>&& scheduling_options =
+        NoFdoSchedulingOptionsFlagsDomain(),
+    fuzztest::Domain<CodegenFlagsProto>&& codegen_options =
+        CodegenFlagsDomain());
+namespace internal {
+std::string UnwrapStatusOrVerilog(
+    const absl::StatusOr<ScheduleAndCodegenResult>& verilog);
+absl::StatusOr<ScheduleAndCodegenResult> DoGenerateVerilog(
+    VerilogGenerator verilog);
+}  // namespace internal
+
 }  // namespace xls
 
 #endif  // XLS_FUZZER_IR_FUZZER_VERILOG_FUZZ_DOMAIN_H_