From d0a535e3e098123298ba2b4fb31313ef5ba1c3ba Mon Sep 17 00:00:00 2001 From: Christian Zentgraf Date: Thu, 29 Aug 2024 17:58:03 -0400 Subject: [PATCH] Refactor setup scripts --- .github/workflows/linux-build-clang.yml | 55 +++++++++++++++++++++++++ .github/workflows/linux-build-gcc.yml | 53 ++++++++++++++++++++++++ .github/workflows/linux-build.yml | 43 ++++++------------- CMakeLists.txt | 12 +++--- scripts/setup-ubuntu.sh | 2 +- velox/dwio/common/tests/CMakeLists.txt | 15 +++++++ 6 files changed, 143 insertions(+), 37 deletions(-) create mode 100644 .github/workflows/linux-build-clang.yml create mode 100644 .github/workflows/linux-build-gcc.yml diff --git a/.github/workflows/linux-build-clang.yml b/.github/workflows/linux-build-clang.yml new file mode 100644 index 0000000000000..0e7dc55a1427e --- /dev/null +++ b/.github/workflows/linux-build-clang.yml @@ -0,0 +1,55 @@ +# Copyright (c) Facebook, Inc. and its affiliates. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: Linux Build using Clang + +on: + push: + branches: + - "main" + paths: + - "velox/**" + - "!velox/docs/**" + - "CMakeLists.txt" + - "CMake/**" + - "third_party/**" + - "scripts/setup-ubuntu.sh" + - "scripts/setup-helper-functions.sh" + - ".github/workflows/linux-build.yml" + - ".github/workflows/linux-build-clang.yml" + + pull_request: + paths: + - "velox/**" + - "!velox/docs/**" + - "CMakeLists.txt" + - "CMake/**" + - "third_party/**" + - "scripts/setup-ubuntu.sh" + - "scripts/setup-helper-functions.sh" + - ".github/workflows/linux-build.yml" + - ".github/workflows/linux-build-clang.yml" + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.repository }}-${{ github.head_ref || github.sha }} + cancel-in-progress: true + +jobs: + ubuntu-clang: + uses: ./.github/workflows/linux-build.yml + with: + use-clang: 'true' diff --git a/.github/workflows/linux-build-gcc.yml b/.github/workflows/linux-build-gcc.yml new file mode 100644 index 0000000000000..623e4991852e5 --- /dev/null +++ b/.github/workflows/linux-build-gcc.yml @@ -0,0 +1,53 @@ +# Copyright (c) Facebook, Inc. and its affiliates. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: Linux Build using GCC + +on: + push: + branches: + - "main" + paths: + - "velox/**" + - "!velox/docs/**" + - "CMakeLists.txt" + - "CMake/**" + - "third_party/**" + - "scripts/setup-ubuntu.sh" + - "scripts/setup-helper-functions.sh" + - ".github/workflows/linux-build.yml" + - ".github/workflows/linux-build-gcc.yml" + + pull_request: + paths: + - "velox/**" + - "!velox/docs/**" + - "CMakeLists.txt" + - "CMake/**" + - "third_party/**" + - "scripts/setup-ubuntu.sh" + - "scripts/setup-helper-functions.sh" + - ".github/workflows/linux-build.yml" + - ".github/workflows/linux-build-gcc.yml" + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.repository }}-${{ github.head_ref || github.sha }} + cancel-in-progress: true + +jobs: + ubuntu-gcc: + uses: ./.github/workflows/linux-build.yml diff --git a/.github/workflows/linux-build.yml b/.github/workflows/linux-build.yml index f2db425f37e17..ba89c10a0a79d 100644 --- a/.github/workflows/linux-build.yml +++ b/.github/workflows/linux-build.yml @@ -15,36 +15,13 @@ name: Linux Build on: - push: - branches: - - "main" - paths: - - "velox/**" - - "!velox/docs/**" - - "CMakeLists.txt" - - "CMake/**" - - "third_party/**" - - "scripts/setup-ubuntu.sh" - - "scripts/setup-helper-functions.sh" - - ".github/workflows/linux-build.yml" - - pull_request: - paths: - - "velox/**" - - "!velox/docs/**" - - "CMakeLists.txt" - - "CMake/**" - - "third_party/**" - - "scripts/setup-ubuntu.sh" - - "scripts/setup-helper-functions.sh" - - ".github/workflows/linux-build.yml" - -permissions: - contents: read - -concurrency: - group: ${{ github.workflow }}-${{ github.repository }}-${{ github.head_ref || github.sha }} - cancel-in-progress: true + workflow_call: + inputs: + use-clang: + description: 'Use Clang to compile the project.' + default: '' + required: false + type: string jobs: adapters: @@ -64,6 +41,7 @@ jobs: xsimd_SOURCE: BUNDLED Arrow_SOURCE: AUTO CUDA_VERSION: "12.4" + USE_CLANG: "${{ inputs.use-clang }}" steps: - uses: actions/checkout@v4 @@ -110,6 +88,7 @@ jobs: "-DVELOX_ENABLE_GPU=ON" "-DVELOX_MONO_LIBRARY=ON" ) + if [[ "${USE_CLANG}" = "true" ]]; then scripts/setup-centos9.sh install_clang15; CC=/usr/bin/clang-15; CXX=/usr/bin/clang++-15; CUDA_FLAGS="-ccbin /usr/lib64/llvm15/bin/clang++-15"; fi make release EXTRA_CMAKE_FLAGS="${EXTRA_CMAKE_FLAGS[*]}" - name: Ccache after @@ -136,6 +115,7 @@ jobs: name: "Ubuntu debug with resolve_dependency" env: CCACHE_DIR: "${{ github.workspace }}/.ccache" + USE_CLANG: "${{ inputs.use-clang }}" defaults: run: shell: bash @@ -171,7 +151,8 @@ jobs: MAKEFLAGS: "NUM_THREADS=8 MAX_HIGH_MEM_JOBS=4 MAX_LINK_JOBS=3" EXTRA_CMAKE_FLAGS: "-DVELOX_ENABLE_ARROW=ON -DVELOX_ENABLE_PARQUET=ON" run: | - make debug + if [[ "${USE_CLANG}" = "true" ]]; then export CC=/usr/bin/clang-15; export CXX=/usr/bin/clang++-15; fi + make debug - name: CCache after run: | diff --git a/CMakeLists.txt b/CMakeLists.txt index 7f7cbc92f250f..5a2da1fa29b10 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -212,11 +212,6 @@ if(${VELOX_FORCE_COLORED_OUTPUT}) endif() endif() -if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" - AND "${CMAKE_CXX_COMPILER_VERSION}" VERSION_GREATER_EQUAL 15) - set(CMAKE_EXE_LINKER_FLAGS "-latomic") -endif() - # At the moment we prefer static linking but by default cmake looks for shared # libs first. This will still fallback to shared libs when static ones are not # found @@ -366,6 +361,13 @@ if(${VELOX_ENABLE_GPU}) include_directories("${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES}") endif() +# Set after the test of the CUDA compiler. Otherwise, the test fails with +# -latomic not found because it is added right after the compiler exe. +if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" + AND "${CMAKE_CXX_COMPILER_VERSION}" VERSION_GREATER_EQUAL 15) + set(CMAKE_EXE_LINKER_FLAGS "-latomic") +endif() + set(CMAKE_EXPORT_COMPILE_COMMANDS ON) set(BOOST_INCLUDE_LIBRARIES diff --git a/scripts/setup-ubuntu.sh b/scripts/setup-ubuntu.sh index e40bba1d99232..f9b02b62c8cc2 100755 --- a/scripts/setup-ubuntu.sh +++ b/scripts/setup-ubuntu.sh @@ -48,7 +48,7 @@ function install_clang15 { fi CLANG_PACKAGE_LIST=clang-15 if [[ ${VERSION} =~ "22.04" ]]; then - CLANG_PACKAGE_LIST=${CLANG_PACKAGE_LIST} gcc-12 g++-12 libc++-12-dev + CLANG_PACKAGE_LIST="${CLANG_PACKAGE_LIST} gcc-12 g++-12 libc++-12-dev" fi ${SUDO} apt install ${CLANG_PACKAGE_LIST} -y } diff --git a/velox/dwio/common/tests/CMakeLists.txt b/velox/dwio/common/tests/CMakeLists.txt index f413dcdab12e9..fcadefeda8ec2 100644 --- a/velox/dwio/common/tests/CMakeLists.txt +++ b/velox/dwio/common/tests/CMakeLists.txt @@ -14,6 +14,21 @@ add_subdirectory(utils) +# There is an issue with the VTT symbol for the InlineExecutor from folly when +# building on Linux with Clang15. It is not created and results in a SEGV when +# attempting to construct the InlineExecutor. nm output shows no address for the +# "v VTT for folly::InlineExecutor" symbol while for a debug build it shows +# "000000000185d3e0 V VTT for folly::InlineExecutor" symbol. It seems to have +# been optimized away by Clang15 and may be some kind of bug. Changing the +# optimization level to 0 results in proper creation/linkage and successful +# execution of the test. Review if this is still necessary when upgrading Clang. +if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") + set_property( + SOURCE ParallelForTest.cpp + APPEND + PROPERTY COMPILE_OPTIONS -O0) +endif() + add_executable( velox_dwio_common_test BitConcatenationTest.cpp