Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build Errors on Mac OS #849

Closed
rachitnigam opened this issue Feb 1, 2023 · 12 comments
Closed

Build Errors on Mac OS #849

rachitnigam opened this issue Feb 1, 2023 · 12 comments
Labels
build Related to build flow, build system, or build macros

Comments

@rachitnigam
Copy link

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
Copy link
Author

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
Copy link
Author

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

@rachitnigam
Copy link
Author

rachitnigam commented Feb 1, 2023

Okay, using the patch from hdl/bazel_rules_hdl#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)

@rachitnigam rachitnigam changed the title Build Error: no matching function call to StrSplit Build Errors on Mac OS Feb 1, 2023
@meheff
Copy link
Collaborator

meheff commented Feb 1, 2023

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
Copy link
Author

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
Copy link
Author

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

@rachitnigam
Copy link
Author

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
Copy link
Collaborator

cdleary commented Feb 2, 2023

@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.

@cdleary cdleary added the build Related to build flow, build system, or build macros label Feb 15, 2023
@makslevental
Copy link

makslevental commented Mar 19, 2023

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
Copy link
Member

proppy commented Jan 10, 2024

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

@proppy proppy closed this as completed Jan 10, 2024
@makslevental
Copy link

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
Copy link
Member

proppy commented Jan 10, 2024

@makslevental I think the plan of record is to revisit the approach described in one of the paragraph of #108 (comment) w/ higher level CFFI bindings filed #1256 to track this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Related to build flow, build system, or build macros
Projects
None yet
Development

No branches or pull requests

5 participants