[c_api] Public API for DSLX steps w/ warning knobs.

google · Feb 7, 2025 · ca4c7d8 · ca4c7d8
1 parent 2763b25
commit ca4c7d8
Show file tree

Hide file tree

Showing 15 changed files with 297 additions and 47 deletions.
diff --git a/xls/contrib/mlir/tools/xls_translate/xls_translate_from_mlir.cc b/xls/contrib/mlir/tools/xls_translate/xls_translate_from_mlir.cc
@@ -1006,9 +1006,12 @@ FailureOr<PackageInfo> importDslxInstantiation(
 
   // Note: using a different pathname here else XLS considers this a circular
   // import.
-  package_string_or =
-      ::xls::ConvertDslxToIr(dslx, "<instantiated module>", module_name,
-                             stdlib_path, additional_search_paths);
+  ::xls::ConvertDslxToIrOptions options = {
+      .dslx_stdlib_path = stdlib_path,
+      .additional_search_paths = additional_search_paths,
+  };
+  package_string_or = ::xls::ConvertDslxToIr(dslx, "<instantiated module>",
+                                             module_name, options);
   if (!package_string_or.ok()) {
     llvm::errs() << "Failed to convert DSLX to IR: "
                  << package_string_or.status().message() << "\n";
@@ -1770,7 +1773,9 @@ absl::StatusOr<std::shared_ptr<const Package>> DslxPackageCache::import(
     return it->second;
   }
   absl::StatusOr<std::string> package_string_or = ::xls::ConvertDslxPathToIr(
-      fileName, ::xls::GetDefaultDslxStdlibPath(), {});
+      fileName, ::xls::ConvertDslxToIrOptions{
+                    .dslx_stdlib_path = ::xls::GetDefaultDslxStdlibPath(),
+                });
   if (!package_string_or.ok()) {
     return package_string_or.status();
   }

diff --git a/xls/dslx/ir_convert/convert_options.h b/xls/dslx/ir_convert/convert_options.h
@@ -40,10 +40,10 @@ struct ConvertOptions {
   // Should warnings be treated as errors?
   bool warnings_as_errors = true;
 
-  // Set of warnings that are enabled.
+  // Set of warnings used in DSLX typechecking.
   //
   // Note that this is only used in IR conversion routines that do typechecking.
-  WarningKindSet enabled_warnings = kDefaultWarningsSet;
+  WarningKindSet warnings = kDefaultWarningsSet;
 
   // Should #[test] and #[test_proc] entities be emitted to IR.
   bool convert_tests = false;

diff --git a/xls/dslx/ir_convert/ir_converter.cc b/xls/dslx/ir_convert/ir_converter.cc
@@ -590,9 +590,9 @@ absl::StatusOr<PackageConversionData> ConvertFilesToPackage(
         "path to know where to resolve the entry function");
   }
   for (std::string_view path : paths) {
-    ImportData import_data(CreateImportData(
-        stdlib_path, dslx_paths, convert_options.enabled_warnings,
-        std::make_unique<RealFilesystem>()));
+    ImportData import_data(
+        CreateImportData(stdlib_path, dslx_paths, convert_options.warnings,
+                         std::make_unique<RealFilesystem>()));
     XLS_ASSIGN_OR_RETURN(std::string text,
                          import_data.vfs().GetFileContents(path));
     XLS_ASSIGN_OR_RETURN(std::string module_name, PathToName(path));

diff --git a/xls/dslx/ir_convert/ir_converter_main.cc b/xls/dslx/ir_convert/ir_converter_main.cc
@@ -111,7 +111,7 @@ absl::Status RealMain(absl::Span<const std::string_view> paths) {
       .emit_fail_as_assert = emit_fail_as_assert,
       .verify_ir = verify_ir,
       .warnings_as_errors = warnings_as_errors,
-      .enabled_warnings = warnings,
+      .warnings = warnings,
       .convert_tests = convert_tests,
       .default_fifo_config = default_fifo_config,
   };

diff --git a/xls/dslx/run_routines/ir_test_runner.cc b/xls/dslx/run_routines/ir_test_runner.cc
@@ -204,7 +204,7 @@ absl::StatusOr<std::unique_ptr<AbstractParsedTestRunner>> MakeRunner(
       .emit_fail_as_assert = true,
       .verify_ir = false,
       .warnings_as_errors = false,
-      .enabled_warnings = kNoWarningsSet,
+      .warnings = kNoWarningsSet,
       .convert_tests = true,
   };
   absl::flat_hash_map<std::string, std::unique_ptr<Package>> packages;

diff --git a/xls/dslx/warning_collector.h b/xls/dslx/warning_collector.h
@@ -48,6 +48,8 @@ class WarningCollector {
 
   const std::vector<Entry>& warnings() const { return warnings_; }
 
+  bool empty() const { return warnings_.empty(); }
+
  private:
   const WarningKindSet enabled_;
   std::vector<Entry> warnings_;

diff --git a/xls/public/c_api.cc b/xls/public/c_api.cc
@@ -58,39 +58,116 @@ void xls_init_xls(const char* usage, int argc, char* argv[]) {
   (void)(xls::InitXls(usage, argc, argv));
 }
 
+bool xls_convert_dslx_to_ir_with_warnings(
+    const char* dslx, const char* path, const char* module_name,
+    const char* dslx_stdlib_path, const char* additional_search_paths[],
+    size_t additional_search_paths_count, const char* enable_warnings[],
+    size_t enable_warnings_count, const char* disable_warnings[],
+    size_t disable_warnings_count, bool warnings_as_errors,
+    char*** warnings_out, size_t* warnings_out_count, char** error_out,
+    char** ir_out) {
+  CHECK(dslx != nullptr);
+  CHECK(path != nullptr);
+  CHECK(dslx_stdlib_path != nullptr);
+  CHECK(error_out != nullptr);
+
+  std::vector<std::filesystem::path> additional_search_paths_cpp =
+      xls::ToCppPaths(additional_search_paths, additional_search_paths_count);
+  std::vector<std::string_view> enable_warnings_cpp =
+      xls::ToCppStrings(enable_warnings, enable_warnings_count);
+  std::vector<std::string_view> disable_warnings_cpp =
+      xls::ToCppStrings(disable_warnings, disable_warnings_count);
+
+  std::vector<std::string> warnings_out_cpp;
+
+  const xls::ConvertDslxToIrOptions options = {
+      .dslx_stdlib_path = dslx_stdlib_path,
+      .additional_search_paths = additional_search_paths_cpp,
+      .enable_warnings = enable_warnings_cpp,
+      .disable_warnings = disable_warnings_cpp,
+      .warnings_as_errors = warnings_as_errors,
+      .warnings_out = &warnings_out_cpp,
+  };
+
+  absl::StatusOr<std::string> result =
+      xls::ConvertDslxToIr(dslx, path, module_name, options);
+
+  if (warnings_out != nullptr && !warnings_out_cpp.empty()) {
+    xls::ToOwnedCStrings(warnings_out_cpp, warnings_out, warnings_out_count);
+  } else if (warnings_out_count != nullptr) {
+    *warnings_out_count = warnings_out_cpp.size();
+  }
+
+  return xls::ReturnStringHelper(result, error_out, ir_out);
+}
+
 bool xls_convert_dslx_to_ir(const char* dslx, const char* path,
                             const char* module_name,
                             const char* dslx_stdlib_path,
                             const char* additional_search_paths[],
                             size_t additional_search_paths_count,
                             char** error_out, char** ir_out) {
-  CHECK(dslx != nullptr);
+  const char* enable_warnings[] = {};
+  const char* disable_warnings[] = {};
+  return xls_convert_dslx_to_ir_with_warnings(
+      dslx, path, module_name, dslx_stdlib_path, additional_search_paths,
+      additional_search_paths_count, enable_warnings, 0, disable_warnings, 0,
+      /*warnings_as_errors=*/false,
+      /*warnings_out=*/nullptr,
+      /*warnings_out_count=*/nullptr, error_out, ir_out);
+}
+
+bool xls_convert_dslx_path_to_ir_with_warnings(
+    const char* path, const char* dslx_stdlib_path,
+    const char* additional_search_paths[], size_t additional_search_paths_count,
+    const char* enable_warnings[], size_t enable_warnings_count,
+    const char* disable_warnings[], size_t disable_warnings_count,
+    bool warnings_as_errors, char*** warnings_out, size_t* warnings_out_count,
+    char** error_out, char** ir_out) {
   CHECK(path != nullptr);
   CHECK(dslx_stdlib_path != nullptr);
   CHECK(error_out != nullptr);
 
   std::vector<std::filesystem::path> additional_search_paths_cpp =
-      xls::ToCpp(additional_search_paths, additional_search_paths_count);
+      xls::ToCppPaths(additional_search_paths, additional_search_paths_count);
+  std::vector<std::string_view> enable_warnings_cpp =
+      xls::ToCppStrings(enable_warnings, enable_warnings_count);
+  std::vector<std::string_view> disable_warnings_cpp =
+      xls::ToCppStrings(disable_warnings, disable_warnings_count);
+
+  std::vector<std::string> warnings_out_cpp;
+
+  const xls::ConvertDslxToIrOptions options = {
+      .dslx_stdlib_path = dslx_stdlib_path,
+      .additional_search_paths = additional_search_paths_cpp,
+      .enable_warnings = enable_warnings_cpp,
+      .disable_warnings = disable_warnings_cpp,
+      .warnings_as_errors = warnings_as_errors,
+      .warnings_out = &warnings_out_cpp,
+  };
+  absl::StatusOr<std::string> result = xls::ConvertDslxPathToIr(path, options);
+
+  if (warnings_out != nullptr && !warnings_out_cpp.empty()) {
+    xls::ToOwnedCStrings(warnings_out_cpp, warnings_out, warnings_out_count);
+  } else if (warnings_out_count != nullptr) {
+    *warnings_out_count = warnings_out_cpp.size();
+  }
 
-  absl::StatusOr<std::string> result = xls::ConvertDslxToIr(
-      dslx, path, module_name, dslx_stdlib_path, additional_search_paths_cpp);
   return xls::ReturnStringHelper(result, error_out, ir_out);
 }
 
 bool xls_convert_dslx_path_to_ir(const char* path, const char* dslx_stdlib_path,
                                  const char* additional_search_paths[],
                                  size_t additional_search_paths_count,
                                  char** error_out, char** ir_out) {
-  CHECK(path != nullptr);
-  CHECK(dslx_stdlib_path != nullptr);
-  CHECK(error_out != nullptr);
-
-  std::vector<std::filesystem::path> additional_search_paths_cpp =
-      xls::ToCpp(additional_search_paths, additional_search_paths_count);
-
-  absl::StatusOr<std::string> result = xls::ConvertDslxPathToIr(
-      path, dslx_stdlib_path, additional_search_paths_cpp);
-  return xls::ReturnStringHelper(result, error_out, ir_out);
+  const char* enable_warnings[] = {};
+  const char* disable_warnings[] = {};
+  return xls_convert_dslx_path_to_ir_with_warnings(
+      path, dslx_stdlib_path, additional_search_paths,
+      additional_search_paths_count, enable_warnings, 0, disable_warnings, 0,
+      /*warnings_as_errors=*/false,
+      /*warnings_out=*/nullptr,
+      /*warnings_out_count=*/nullptr, error_out, ir_out);
 }
 
 bool xls_optimize_ir(const char* ir, const char* top, char** error_out,
@@ -465,6 +542,17 @@ void xls_c_str_free(char* c_str) {
   free(c_str);
 }
 
+void xls_c_strs_free(char** c_strs, size_t count) {
+  if (c_strs == nullptr) {
+    return;
+  }
+
+  for (size_t i = 0; i < count; ++i) {
+    free(c_strs[i]);
+  }
+  delete[] c_strs;
+}
+
 bool xls_value_to_string(const struct xls_value* v, char** string_out) {
   CHECK(v != nullptr);
   CHECK(string_out != nullptr);

diff --git a/xls/public/c_api.h b/xls/public/c_api.h
@@ -60,11 +60,34 @@ bool xls_convert_dslx_to_ir(const char* dslx, const char* path,
                             size_t additional_search_paths_count,
                             char** error_out, char** ir_out);
 
+// As above, but also takes `enable_warnings` and `disable_warnings` which
+// are arrays of warning names to enable or disable respectively against the
+// default warning set.
+//
+// Note: `warnings_out` should be freed via `xls_c_strs_free`.
+bool xls_convert_dslx_to_ir_with_warnings(
+    const char* dslx, const char* path, const char* module_name,
+    const char* dslx_stdlib_path, const char* additional_search_paths[],
+    size_t additional_search_paths_count, const char* enable_warnings[],
+    size_t enable_warnings_count, const char* disable_warnings[],
+    size_t disable_warnings_count, bool warnings_as_errors,
+    char*** warnings_out, size_t* warnings_out_count, char** error_out,
+    char** ir_out);
+
 bool xls_convert_dslx_path_to_ir(const char* path, const char* dslx_stdlib_path,
                                  const char* additional_search_paths[],
                                  size_t additional_search_paths_count,
                                  char** error_out, char** ir_out);
 
+// Note: `warnings_out` should be freed via `xls_c_strs_free`.
+bool xls_convert_dslx_path_to_ir_with_warnings(
+    const char* path, const char* dslx_stdlib_path,
+    const char* additional_search_paths[], size_t additional_search_paths_count,
+    const char* enable_warnings[], size_t enable_warnings_count,
+    const char* disable_warnings[], size_t disable_warnings_count,
+    bool warnings_as_errors, char*** warnings_out, size_t* warnings_out_count,
+    char** error_out, char** ir_out);
+
 bool xls_optimize_ir(const char* ir, const char* top, char** error_out,
                      char** ir_out);
 
@@ -237,6 +260,9 @@ void xls_package_free(struct xls_package* p);
 // just call `free` directly).
 void xls_c_str_free(char* c_str);
 
+// Frees an array of C strings that were provided by this XLS public API.
+void xls_c_strs_free(char** c_strs, size_t count);
+
 // Returns a string representation of the given IR package `p`.
 bool xls_package_to_string(const struct xls_package* p, char** string_out);
 

diff --git a/xls/public/c_api_dslx.cc b/xls/public/c_api_dslx.cc
@@ -67,7 +67,7 @@ struct xls_dslx_import_data* xls_dslx_import_data_create(
     size_t additional_search_paths_count) {
   std::filesystem::path cpp_stdlib_path{dslx_stdlib_path};
   std::vector<std::filesystem::path> cpp_additional_search_paths =
-      xls::ToCpp(additional_search_paths, additional_search_paths_count);
+      xls::ToCppPaths(additional_search_paths, additional_search_paths_count);
   xls::dslx::ImportData import_data = CreateImportData(
       cpp_stdlib_path, cpp_additional_search_paths, xls::dslx::kAllWarningsSet,
       std::make_unique<xls::dslx::RealFilesystem>());

diff --git a/xls/public/c_api_impl_helpers.cc b/xls/public/c_api_impl_helpers.cc
@@ -29,8 +29,9 @@
 
 namespace xls {
 
-std::vector<std::filesystem::path> ToCpp(const char* additional_search_paths[],
-                                         size_t additional_search_paths_count) {
+std::vector<std::filesystem::path> ToCppPaths(
+    const char* additional_search_paths[],
+    size_t additional_search_paths_count) {
   std::vector<std::filesystem::path> additional_search_paths_cpp;
   additional_search_paths_cpp.reserve(additional_search_paths_count);
   for (size_t i = 0; i < additional_search_paths_count; ++i) {
@@ -41,10 +42,29 @@ std::vector<std::filesystem::path> ToCpp(const char* additional_search_paths[],
   return additional_search_paths_cpp;
 }
 
+std::vector<std::string_view> ToCppStrings(const char* strings[],
+                                           size_t strings_count) {
+  std::vector<std::string_view> strings_cpp;
+  strings_cpp.reserve(strings_count);
+  for (size_t i = 0; i < strings_count; ++i) {
+    const char* string = strings[i];
+    CHECK(string != nullptr);
+    strings_cpp.push_back(string);
+  }
+  return strings_cpp;
+}
+
 char* ToOwnedCString(std::string_view s) { return strndup(s.data(), s.size()); }
 
-// Helper function that we can use to adapt to the common C API pattern when
-// we're returning an `absl::StatusOr<std::string>` value.
+void ToOwnedCStrings(absl::Span<const std::string> cpp, char*** c_out,
+                     size_t* c_out_count) {
+  *c_out = new char*[cpp.size()];
+  *c_out_count = cpp.size();
+  for (size_t i = 0; i < cpp.size(); ++i) {
+    (*c_out)[i] = ToOwnedCString(cpp[i]);
+  }
+}
+
 bool ReturnStringHelper(absl::StatusOr<std::string>& to_return,
                         char** error_out, char** value_out) {
   if (to_return.ok()) {

diff --git a/xls/public/c_api_impl_helpers.h b/xls/public/c_api_impl_helpers.h
@@ -46,10 +46,20 @@ bool ReturnStringHelper(absl::StatusOr<std::string>& to_return,
 // caller.
 char* ToOwnedCString(std::string_view s);
 
+// Populates `c_out` with a C-style array of strings. It is a "char* array
+// pointer", and it is populated with `cpp.size()` elements. Each element is a
+// C-style string that is a `strdup` result.
+void ToOwnedCStrings(absl::Span<const std::string> cpp, char*** c_out,
+                     size_t* c_out_count);
+
 // Converts the C representation of filesystem search paths into a C++
 // representation.
-std::vector<std::filesystem::path> ToCpp(const char* additional_search_paths[],
-                                         size_t additional_search_paths_count);
+std::vector<std::filesystem::path> ToCppPaths(const char* paths[],
+                                              size_t paths_count);
+
+// Converts the C representation of warnings into a C++ representation.
+std::vector<std::string_view> ToCppStrings(const char* strings[],
+                                           size_t strings_count);
 
 }  // namespace xls
 

diff --git a/xls/public/c_api_symbols.txt b/xls/public/c_api_symbols.txt
@@ -21,8 +21,11 @@ xls_bits_umul
 xls_bits_width_slice
 xls_bits_xor
 xls_c_str_free
+xls_c_strs_free
 xls_convert_dslx_path_to_ir
+xls_convert_dslx_path_to_ir_with_warnings
 xls_convert_dslx_to_ir
+xls_convert_dslx_to_ir_with_warnings
 xls_dslx_colon_ref_get_attr
 xls_dslx_colon_ref_resolve_import_subject
 xls_dslx_constant_def_get_name