Skip to content

Commit

Permalink
Use separate Bazel invocation for TF integration binaries (iree-org#4836
Browse files Browse the repository at this point in the history
)

Rather than having CMake invoke Bazel itself, require that the
necessary binaries be built prior to running CMake and then import
them.

This is similar to what iree-org#4584 did
in the Android build and has similar advantages: the user can get these
binaries in a variety of ways: building with their chosen config (e.g.
with local or remote caching), installing prebuilt binaries,  etc.

It also is significantly more clear what's going on IMO. Having CMake
invoke Bazel has caused a fair amount of confusion.

Incidentally, I'm pretty sure this increases test coverage by enabling
TF tests that were apparently not getting run.

Tested:
In addition to the CI, ran all the release distribution commands
locally and confirmed they still exited successfully.
  • Loading branch information
GMNGeoffrey authored Feb 26, 2021
1 parent 3998012 commit f03c3c1
Show file tree
Hide file tree
Showing 21 changed files with 297 additions and 341 deletions.
2 changes: 1 addition & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.

# Import the main bazelrc config. This is in a separate file so that it's
# possible to turn off all user bazelrc options by specifying
# possible to turn off some or all user and system bazelrc options by specifying
# `--nosystem_rc --nohome_rc --noworkspace_rc --bazelrc=build_tools/bazel/iree.bazelrc`
try-import %workspace%/build_tools/bazel/iree.bazelrc

Expand Down
7 changes: 7 additions & 0 deletions .github/workflows/build_package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,13 @@ jobs:
EOF
cat ./main_checkout/version_info.json
- name: Configure Bazel
if: matrix.needs_bazel
shell: bash
run: |
cd main_checkout
python ./configure_bazel.py
# The main distribution builds the main binary package, tests and compiler
# wheels (which are OS but not python version specific). The latter is
# purely opportunistic: it adds incrementally to the former and would
Expand Down
10 changes: 0 additions & 10 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -370,16 +370,6 @@ if(${IREE_BUILD_PYTHON_BINDINGS})
find_package(Python3 COMPONENTS Interpreter Development REQUIRED)
endif()

#-------------------------------------------------------------------------------
# Bazel setup (conditional on whether features need it)
# Depends on python configuration.
#-------------------------------------------------------------------------------

if(${IREE_ENABLE_TENSORFLOW})
include(configure_bazel)
iree_configure_bazel()
endif()

#-------------------------------------------------------------------------------
# Other dependencies.
#-------------------------------------------------------------------------------
Expand Down
3 changes: 0 additions & 3 deletions bindings/python/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ First perform a normal CMake build/install with the following options:
* `-DCMAKE_INSTALL_PREFIX=...path to install to...` : Sets up installation
prefix.
* `-DIREE_BUILD_PYTHON_BINDINGS=ON` : Enables Python Bindings
* `-DIREE_BUILD_TENSORFLOW_COMPILER=ON` (optional) : Enables building the
TensorFlow compilers (note: requires additional dependencies. see overall
build docs).

Then from the install directory, run:

Expand Down
17 changes: 17 additions & 0 deletions build_tools/bazel/iree.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,20 @@ build:macos_clang --config=generic_clang
build:macos_clang --per_file_copt=tensorflow,iree_tf_compiler@-Wno-unused-variable
build:macos_clang --per_file_copt=tensorflow,iree_tf_compiler@-Wno-range-loop-analysis

###############################################################################
# Additional options for release builds. These do *not* automatically pull in
# the corresponding compiler config (e.g. generic_clang) because this is likely
# to already be set in the user's configured.bazelrc and setting a config twice
# causes flags to be passed twice.
###############################################################################
build:generic_clang_release --compilation_mode=opt
build:generic_clang_release --linkopt=-Wl,--strip-all
build:generic_gcc_release --compilation_mode=opt
build:generic_gcc_release --linkopt=-Wl,--strip-all
# MSVC doesn't store symbols in the binary (they're in separate .pdb files), so
# nothing needs to be stripped.
build:msvc_release --compilation_mode=opt

###############################################################################
# Options for building with address sanitizer.
# https://github.com/google/sanitizers/wiki/AddressSanitizer
Expand Down Expand Up @@ -303,6 +317,9 @@ build:rs --project_id=iree-oss

build:_msvc_base --define=iree_is_msvc=true

# Bazel gets confused and sometimes loses track of Bash.
build:_msvc_base --action_env BAZEL_SH='bash'

# Disable warnings for dependencies. We don't control these.
# absl forces /W3 in their copts, so we exclude them to avoid D9025
build:_msvc_base --per_file_copt=+external,-com_google_absl@/w
Expand Down
204 changes: 0 additions & 204 deletions build_tools/cmake/configure_bazel.cmake

This file was deleted.

Loading

0 comments on commit f03c3c1

Please sign in to comment.