search

google / xls

XLS: Accelerated HW Synthesis
http://google.github.io/xls/
Apache License 2.0
1.2k stars 177 forks source link

Build Errors on Mac OS #849

Closed rachitnigam closed 9 months ago

rachitnigam commented 1 year ago

I'm trying to build XLS from source on a mac os machine with the following command line invocation:

 bazel build -c opt //xls/... --action_env=PYTHON_BIN_PATH=$(which python)

(The additional --action_env is for another error I was running into where the python autoconfiguration was failing because it was trying to access an empty include folder)

I get the following build error:

ERROR: /Users/rachitnigam/git/xls/xls/common/BUILD:50:11: Compiling xls/common/case_converters.cc [for tool] failed: (Exit 1): wrapped_clang_pp failed: error executing command (from target //xls/common:case_converters) external/local_config_cc/wrapped_clang_pp '-D_FORTIFY_SOURCE=1' -fstack-protector -fcolor-diagnostics -Wall -Wthread-safety -Wself-assign -fno-omit-frame-pointer -g0 -O2 -DNDEBUG ... (remaining 37 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
xls/common/case_converters.cc:25:7: error: no matching function for call to 'StrSplit'
      absl::StrSplit(input, absl::ByAnyChar("-_"));
      ^~~~~~~~~~~~~~
external/com_google_absl/absl/strings/str_split.h:499:1: note: candidate function template not viable: no known conversion from 'std::string_view' (aka 'basic_string_view<char>') to 'strings_internal::ConvertibleToStringView' for 1st argument
StrSplit(strings_internal::ConvertibleToStringView text, Delimiter d) {
^
external/com_google_absl/absl/strings/str_split.h:512:1: note: candidate template ignored: requirement 'std::is_same<std::string_view &, std::string>::value || std::is_same<std::string_view &, const std::string>::value' was not satisfied [with Delimiter = absl::ByAnyChar, StringType = std::string_view &]
StrSplit(StringType&& text, Delimiter d) {
^
external/com_google_absl/absl/strings/str_split.h:523:1: note: candidate function template not viable: requires 3 arguments, but 2 were provided
StrSplit(strings_internal::ConvertibleToStringView text, Delimiter d,
^
external/com_google_absl/absl/strings/str_split.h:537:1: note: candidate function template not viable: requires 3 arguments, but 2 were provided
StrSplit(StringType&& text, Delimiter d, Predicate p) {
^

It seems that for some reason it can neither convert the arguments to the right type to match a string split call, nor match against one of the other overloaded methods.

I'm not profoundly familiar with debugging with bazel so couldn't explore any deeper but would be happy to be linked to other resources and run more commands to investigate

rachitnigam commented 1 year ago

Simply rerunning the above command results in a different error which makes me think that maybe there is something wrong with the configuration? 

In file included from xls/common/status/error_code_to_status.cc:15:
In file included from ./xls/common/status/error_code_to_status.h:21:
./xls/common/status/status_builder.h:587:31: error: no template named 'make_unique' in namespace 'std'; did you mean 'absl::make_unique'?
  if (rep_ == nullptr) rep_ = std::make_unique<Rep>();
                              ^~~~~~~~~~~~~~~~
                              absl::make_unique
external/com_google_absl/absl/memory/memory.h:168:55: note: 'absl::make_unique' declared here
typename memory_internal::MakeUniqueResult<T>::scalar make_unique(

I assume this is because bazel is running with the wrong C++ version where make_unique is not available

rachitnigam commented 1 year ago

Resolved the previous problem by passing --incompatible_enable_cc_toolchain_resolution but ran into: https://github.com/hdl/bazel_rules_hdl/issues/140 (TLDR: the build uses ld -no-pie but mac os ld has the option -no_pie)

rachitnigam commented 1 year ago

Okay, using the patch from https://github.com/hdl/bazel_rules_hdl/pull/141, the above error is fixed but now there is a new error about constant expressions:

xls/fuzzer/ast_generator_test.cc:168:43: error: non-constant-expression cannot be narrowed from type 'uint64_t' (aka 'unsigned long long') to 'std::mersenne_twister_engine<unsigned int, 32, 624, 397, 31, 2567483615, 11, 4294967295, 7, 2636928640, 15, 4022730752, 18, 1812433253>::result_type' (aka 'unsigned int') in initializer list [-Wc++11-narrowing]

or non-deterministically:

ERROR: /Users/rachitnigam/git/xls/xls/uncore_rtl/ice40/BUILD:64:14: declared output 'xls/uncore_rtl/ice40/uart_transmitter_two_bytes_early_cpb4_test.iverilog.out' was not created by genrule. This is probably because the genrule actually didn't create this output, or because the output was a directory and the genrule was run remotely (note that only the contents of declared file outputs are copied from genrules run remotely)
meheff commented 1 year ago

We only build and test XLS on Ubuntu so we can't guarantee much will work on other platforms. The option --incompatible_enable_cc_toolchain_resolution switches to building with LLVM which we would like to enable by default (LLVM is used internally at Google), but for some reason using LLVM results in a number of failures related to pybind. So, just as a warning, you may encounter a number of non-trivial issues getting everything working on MacOS. Wish we could be more helpful here.

In any case, the immediate build problem you point to looks like unintential narrowing which we'll fix.

rachitnigam commented 1 year ago

Thanks @meheff, I'll take a look! Also CC @cdleary since I was chatting with him about this issue as well. It seems my most recent failure does indeed have to do with pybind uses inside the fuzzer

rachitnigam commented 1 year ago

Another note: need to brew install coreutils to get sha256 command which is not available by default on Mac OS

rachitnigam commented 1 year ago

Unfortunately the pybind problems might be hard to get around because the tests seem to fail:

Executing tests from //xls/dslx:value_test
-----------------------------------------------------------------------------
Traceback (most recent call last):
  File "/private/var/tmp/_bazel_rachitnigam/b3261ecaf5b6ec09d8051d16006b13a8/sandbox/darwin-sandbox/21190/execroot/com_google_xls/bazel-out/darwin-opt/bin/xls/dslx/value_test.runfiles/com_google_xls/xls/dslx/value_test.py", line 21, in <module>
    from xls.dslx.python import interp_value
ImportError: dlopen(/private/var/tmp/_bazel_rachitnigam/b3261ecaf5b6ec09d8051d16006b13a8/sandbox/darwin-sandbox/21190/execroot/com_google_xls/bazel-out/darwin-opt/bin/xls/dslx/value_test.runfiles/com_google_xls/xls/dslx/python/interp_value.so, 0x0002): Library not loaded: 'bazel-out/darwin-opt/bin/external/pybind11_abseil/pybind11_abseil/libimport_status_module.dylib'
  Referenced from: '/private/var/tmp/_bazel_rachitnigam/b3261ecaf5b6ec09d8051d16006b13a8/execroot/com_google_xls/bazel-out/darwin-opt/bin/xls/dslx/python/interp_value.so'
  Reason: tried: 'bazel-out/darwin-opt/bin/external/pybind11_abseil/pybind11_abseil/libimport_status_module.dylib' (no such file), '/usr/local/lib/libimport_status_module.dylib' (no such file), '/usr/lib/libimport_status_module.dylib' (no such file)

From running the command:

bazel test -c opt --action_env=PYTHON_BIN_PATH=$(which python) --incompatible_enable_cc_toolchain_resolution //xls/dslx:all
cdleary commented 1 year ago

@rachitnigam Python targets will have issues with the hermetic toolchain, but all the cc_binary / cc_test targets should be ok. (That one you're running is looks like a Python target even though it doesn't have Python explicitly in the target name.)

I think changing linkstatic in this genrule when you're using the hermetic toolchain (Clang) could also be something to try for the Python targets: https://sourcegraph.com/github.com/google/xls/-/blob/dependency_support/pybind11/pybind11.bzl?L27 (You can see issue #808 for debugging info if you happen to be curious.) Sorry we haven't finished debugging the hermetic toolchain build yet. As we discussed yesterday, we're happy to take patches for this build, but on my macbook I run in a docker container to make sure I get supported build flow as it's harder to regression test OS X builds.

makslevental commented 1 year ago

TL;DR

On my M1 (12.5) I am able to able to build all of the py_library and xls_pybind_extension targets (against python=3.9.16):

bazel build -c opt --sandbox_debug \
  --extra_toolchains=@llvm_toolchain//:cc-toolchain-aarch64-darwin \
  //xls/dslx:interpreter_main \
  //xls/dslx/ir_convert:ir_converter_main \
  //xls/tools:opt_main \
  //xls/tools:codegen_main \
  //xls/tools:proto_to_dslx_main \
  //xls/tools:package_bazel_build \
  //xls/public/python:builder \
  //xls/common:xls_error \
  //xls/common:memoize \
  //xls/common:runfiles \
  //xls/common:multiprocess \
  //xls/common:test_base \
  //xls/common:gfile \
  //xls/common:check_simulator \
  //xls/common:revision \
  //xls/common/file/python:filesystem \
  //xls/delay_model:delay_model \
  //xls/delay_model:op_module_generator \
  //xls/synthesis:client_credentials \
  //xls/synthesis:synthesis_utils \
  //xls/synthesis:timing_characterization_client \
  //xls/solvers/python:lec_characterizer \
  //xls/ir:op_specification_typechecked \
  //xls/fuzzer:cli_helpers \
  //xls/fuzzer:run_fuzz \
  //xls/fuzzer:sample_runner \
  //xls/fuzzer:run_fuzz_multiprocess_lib \
  //xls/experimental/smtlib:flags_checks \
  //xls/experimental/smtlib:n_bit_nested_add_generator \
  //xls/experimental/smtlib:n_bit_nested_mul_generator \
  //xls/experimental/smtlib:n_bit_nested_shift_generator \
  //xls/experimental/smtlib:solvers_op_comparison_functions \
  //xls/public/python:runtime_build_actions \
  //xls/common/python:init_xls \
  //xls/codegen/python:module_signature \
  //xls/codegen/python:pipeline_generator \
  //xls/simulation/python:module_simulator \
  //xls/solvers/python:z3_lec \
  //xls/ir/python:bits \
  //xls/ir/python:fileno \
  //xls/ir/python:function \
  //xls/ir/python:function_builder \
  //xls/ir/python:ir_parser \
  //xls/ir/python:lsb_or_msb \
  //xls/ir/python:package \
  //xls/ir/python:source_location \
  //xls/ir/python:type \
  //xls/ir/python:op \
  //xls/ir/python:value \
  //xls/ir/python:verifier \
  //xls/passes/python:standard_pipeline \
  //xls/fuzzer/python:cpp_ast_generator \
  //xls/fuzzer/python:cpp_sample \
  //xls/dslx/python:ast \
  //xls/dslx/python:interp_value \
  //xls/dslx/python:create_import_data \
  //xls/dslx/python:parse_and_typecheck \
  //xls/dslx/python:interpreter \
  //xls/tools/python:eval_helpers \
  //xls/visualization/ir_viz/python:ir_to_json

using this patch

diff --git a/.bazelrc b/.bazelrc
index 077f5fa4..7068016e 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -1,10 +1,17 @@
 build --action_env=BAZEL_CXXOPTS=-std=c++17
+build --action_env=PYTHON_BIN_PATH=/Users/mlevental/miniforge3/envs/xls/bin/python
+build --repo_env PYTHON_BIN_PATH=/Users/mlevental/miniforge3/envs/xls/bin/python
 build --cxxopt "-std=c++17"
+build --cxxopt "-Wno-deprecated-builtins"
 build --copt "-Wno-sign-compare"
+build --copt "-Wno-deprecated-builtins"
 build --copt "-Wno-comment"
 build --host_copt "-Wno-sign-compare"
+build --host_copt "-Wno-deprecated-builtins"
 build --host_copt "-Wno-comment"

 # TODO(leary): 2020-09-09 Make it possible to enable this option.
 # Currently m4 doesn't seem to work as a dependency.
 # build --crosstool_top=@llvm_toolchain_llvm//:toolchain
+build --incompatible_enable_cc_toolchain_resolution
diff --git a/dependency_support/pip_requirements.txt b/dependency_support/pip_requirements.txt
index 2adb5d6e..9ebe4bc9 100644
--- a/dependency_support/pip_requirements.txt
+++ b/dependency_support/pip_requirements.txt
@@ -12,5 +12,5 @@ pyyaml==5.4.1
 # Note: numpy and scipy version availability seems to differ between Ubuntu
 # versions that we want to support (e.g. 18.04 vs 20.04), so we accept a
 # range that makes successful installation on those platforms possible.
-numpy>=1.21
-scipy>=1.5.4,<=1.8.1
+numpy
+scipy
diff --git a/xls/common/subprocess.cc b/xls/common/subprocess.cc
index 866e723f..c796db28 100644
--- a/xls/common/subprocess.cc
+++ b/xls/common/subprocess.cc
@@ -19,6 +19,7 @@
 #include <sys/wait.h>
 #include <unistd.h>

+#include <csignal>
 #include <cerrno>
 #include <cstdlib>
 #include <cstring>

Explanation

Of key changes (in my understanding):

  1. build --incompatible_enable_cc_toolchain_resolution: default toolchain fails (doesn't support c++17 and BAZEL_USE_CPP_ONLY_TOOLCHAIN fails as well).
  2. +build --cxxopt "-Wno-deprecated-builtins": abseil fails with builtin __has_trivial_destructor is deprecated; use __is_trivially_destructible instead.
  3. unpinning numpy,scipy: lapack issues when compiling scipy from source (for the pinned version) due to missing arm implementations (or something like that).
  4. +#include <csignal>: in the macos stdlib, signal.h isn't transitively included through one of existing headers (hence include it explicitly).

Note, for python=3.11, I also got errors in grpc due to cpython API changes (something like this).

Possibly I did some things wrong (I'm not super familiar with bazel). FYI I would like to get python wheels working (to make prototyping a frontend easier).

Update

By adding 

diff --git a/xls/BUILD b/xls/BUILD
index 2750753e..86ebf444 100644
--- a/xls/BUILD
+++ b/xls/BUILD
@@ -70,3 +70,85 @@ package_group(
     packages = ["//..."],
 )
 # LINT.ThenChange(//xls/BUILD)
+
+
+load("@rules_python//python:packaging.bzl", "py_package")
+load("@rules_python//python:packaging.bzl", "py_wheel")
+
+# Use py_package to collect all transitive dependencies of a target,
+# selecting just the files within a specific python package.
+py_package(
+    name = "pyxls",
+    # Only include these Python packages.
+    packages = [
+"xls.public.python.runtime_build_actions",
+"xls.common.python.init_xls",
+"xls.codegen.python.module_signature",
+"xls.codegen.python.pipeline_generator",
+#"xls.simulation.python.module_simulator",
+#"xls.solvers.python.z3_lec",
+"xls.ir.python.bits",
+"xls.ir.python.fileno",
+"xls.ir.python.function",
+"xls.ir.python.function_builder",
+"xls.ir.python.ir_parser",
+"xls.ir.python.lsb_or_msb",
+"xls.ir.python.package",
+"xls.ir.python.source_location",
+"xls.ir.python.type",
+"xls.ir.python.op",
+"xls.ir.python.value",
+"xls.ir.python.verifier",
+#"xls.passes.python.standard_pipeline",
+"xls.fuzzer.python.cpp_ast_generator",
+"xls.fuzzer.python.cpp_sample",
+"xls.dslx.python.ast",
+"xls.dslx.python.interp_value",
+"xls.dslx.python.create_import_data",
+"xls.dslx.python.parse_and_typecheck",
+"xls.dslx.python.interpreter",
+"xls.tools.python.eval_helpers",
+"xls.visualization.ir_viz.python.ir_to_json",
+    ],
+    deps = [
+"//xls/public/python:runtime_build_actions",
+"//xls/common/python:init_xls",
+"//xls/codegen/python:module_signature",
+"//xls/codegen/python:pipeline_generator",
+#"//xls/simulation/python:module_simulator",
+#"//xls/solvers/python:z3_lec",
+"//xls/ir/python:bits",
+"//xls/ir/python:fileno",
+"//xls/ir/python:function",
+"//xls/ir/python:function_builder",
+"//xls/ir/python:ir_parser",
+"//xls/ir/python:lsb_or_msb",
+"//xls/ir/python:package",
+"//xls/ir/python:source_location",
+"//xls/ir/python:type",
+"//xls/ir/python:op",
+"//xls/ir/python:value",
+"//xls/ir/python:verifier",
+#"//xls/passes/python:standard_pipeline",
+"//xls/fuzzer/python:cpp_ast_generator",
+"//xls/fuzzer/python:cpp_sample",
+"//xls/dslx/python:ast",
+"//xls/dslx/python:interp_value",
+"//xls/dslx/python:create_import_data",
+"//xls/dslx/python:parse_and_typecheck",
+"//xls/dslx/python:interpreter",
+"//xls/tools/python:eval_helpers",
+"//xls/visualization/ir_viz/python:ir_to_json",
+"@pybind11_abseil//pybind11_abseil:absl_casters",
+"@pybind11_abseil//pybind11_abseil:statusor_caster",
+    ],
+)
+
+py_wheel(
+    name = "pyxls_wheel",
+    # Package data. We're building "example_minimal_package-0.0.1-py3-none-any.whl"
+    distribution = "pyxls",
+    python_tag = "py3",
+    version = "0.0.1",
+    deps = [":pyxls"],
+)

then running

bazel build -c opt --sandbox_debug \
  --extra_toolchains=@llvm_toolchain//:cc-toolchain-aarch64-darwin \
  //xls:pyxls_wheel

I get a wheel at bazel-bin/xls/pyxls-0.0.1-py3-none-any.whl. Copying this to a "wheelhouse" and unzipping and then some more "artisanal" copying:

(xls) wheelhouse % unzip pyxls-0.0.1-py3-none-any.whl
...
(xls) wheelhouse % cp -R ../bazel-xls/bazel-out/darwin_arm64-opt/bin/xls/public/python/builder_test.runfiles/pybind11_abseil .
(xls) wheelhouse % cp -R ../bazel-xls/bazel-out/darwin_arm64-opt/bin/_solib_darwin .

and then I am able to get function_builder_test.py to pass:

(xls) wheelhouse % PYTHONPATH=. python function_builder_test.py 
Running tests under Python 3.9.16: /Users/mlevental/miniforge3/envs/xls/bin/python
[ RUN      ] FunctionBuilderTest.test_all_add_methods
[       OK ] FunctionBuilderTest.test_all_add_methods
[ RUN      ] FunctionBuilderTest.test_bvalue_methods
[       OK ] FunctionBuilderTest.test_bvalue_methods
[ RUN      ] FunctionBuilderTest.test_invoke_adder_2_plus_3_eq_5
[       OK ] FunctionBuilderTest.test_invoke_adder_2_plus_3_eq_5
[ RUN      ] FunctionBuilderTest.test_literal_array
[       OK ] FunctionBuilderTest.test_literal_array
[ RUN      ] FunctionBuilderTest.test_match_true
[       OK ] FunctionBuilderTest.test_match_true
[ RUN      ] FunctionBuilderTest.test_simple_build_and_dump_package
[       OK ] FunctionBuilderTest.test_simple_build_and_dump_package
----------------------------------------------------------------------
Ran 6 tests in 0.001s

OK

Obviously I'm doing a whole bunch of things manually that bazel can do for me - would appreciate some advice.

Possibly bazel-bin/xls/tools/package_bazel_build needs to be extended to support packaging the python extensions (currently those that aren't a dep in a py_test, such as xls/ir/python/function.cc/xls/ir/python/function.o, don't get a runfiles_manifest generated and thus aren't package_bazel_build-packagable.

proppy commented 9 months ago

I think we can close this know that we use an hermetic toolchain eb2fb6d81523b09fbd77b63b88381b1ad6c74850, that we removed unused python extension d27e24b7b394d8d5ba1cd3c3666ae97a74a95fd0 and that we have macos runner d5299e9a872b6a88827c0dc3e94e04b8ab57403c.

makslevental commented 9 months ago

I think we can close this know that we use an hermetic toolchain eb2fb6d, that we removed unused python extension d27e24b ...

Sad to hear you removed the python extension :(

proppy commented 9 months ago

@makslevental I think the plan of record is to revisit the approach described in one of the paragraph of https://github.com/google/xls/issues/108#issue-682499715 w/ higher level CFFI bindings filed https://github.com/google/xls/issues/1256 to track this.