Skip to content

Commit

Permalink
Run clang-tidy on macOS runners instead of Linux (halide#7746)
Browse files Browse the repository at this point in the history
* Run clang-tidy on macOS runners instead of LInux

The current macOS runners have twice the RAM and more CPU power.

Also, drive-by change to allow specifying the parallelism that the run-clang-tidy script should use (defaults to nproc)

* Update Generator.cpp

* Update run-clang-tidy.sh

* Update run-clang-tidy.sh
  • Loading branch information
steven-johnson authored and ardier committed Mar 3, 2024
1 parent bbdc051 commit bda9e9a
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 16 deletions.
20 changes: 10 additions & 10 deletions .github/workflows/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,24 @@ jobs:
source: '.'
extensions: 'h,c,cpp'
clangFormatVersion: 16
# As of Aug 2023, the macOS runners have more RAM (14GB vs 7GB) and CPU (3 cores vs 2)
# than the Linux and Windows runners, so let's use those instead, since clang-tidy is
# a bit of a sluggard
check_clang_tidy:
name: Check clang-tidy
runs-on: ubuntu-20.04
runs-on: macos-13
steps:
- uses: actions/checkout@v3
- name: Install clang-tidy
run: |
# from apt.llvm.org
# wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -
sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 15CF4D18AF4F7421
sudo apt-add-repository "deb https://apt.llvm.org/$(lsb_release -sc)/ llvm-toolchain-$(lsb_release -sc)-16 main"
sudo apt-get update
sudo apt-get install llvm-16 clang-16 liblld-16-dev libclang-16-dev clang-tidy-16 ninja-build
brew update
brew install llvm@16 ninja coreutils
- name: Run clang-tidy
run: |
export CC=clang-16
export CXX=clang++-16
export CLANG_TIDY_LLVM_INSTALL_DIR=/usr/lib/llvm-16
export CC=/usr/local/opt/llvm/bin/clang
export CXX=/usr/local/opt/llvm/bin/clang++
export CLANG_TIDY_LLVM_INSTALL_DIR=/usr/local/opt/llvm
export CMAKE_GENERATOR=Ninja
./run-clang-tidy.sh
check_cmake_file_lists:
name: Check CMake file lists
Expand Down
35 changes: 30 additions & 5 deletions run-clang-tidy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,31 @@ set -e

ROOT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"

[[ "$1" != "" && "$1" != "-fix" ]] && echo "The only supported argument is -fix" && exit

FIX=$1
usage() { echo "Usage: $0 [-j MAX_PROCESS_COUNT] [-f]" 1>&2; exit 1; }

J=$(nproc)
FIX=

while getopts ":j:f" o; do
case "${o}" in
j)
J="${OPTARG}"
[[ "${J}" =~ ^[0-9]+$ ]] || ( echo "-j requires an integer argument"; usage )
;;
f)
FIX="-fix"
;;
*)
usage
;;
esac
done
shift $((OPTIND-1))

echo "Using ${J} processes."
if [ -n "${FIX}" ]; then
echo "Operating in -fix mode!"
fi

# We are currently standardized on using LLVM/Clang16 for this script.
# Note that this is totally independent of the version of LLVM that you
Expand Down Expand Up @@ -47,7 +69,8 @@ cmake -DCMAKE_BUILD_TYPE=Debug \
[ -a ${CLANG_TIDY_BUILD_DIR}/compile_commands.json ]

# We must populate the includes directory to check things outside of src/
cmake --build ${CLANG_TIDY_BUILD_DIR} --target HalideIncludes
echo Building HalideIncludes...
cmake --build ${CLANG_TIDY_BUILD_DIR} -j $(nproc) --target HalideIncludes

RUN_CLANG_TIDY=${CLANG_TIDY_LLVM_INSTALL_DIR}/bin/run-clang-tidy

Expand All @@ -57,6 +80,7 @@ RUN_CLANG_TIDY=${CLANG_TIDY_LLVM_INSTALL_DIR}/bin/run-clang-tidy
# Skip DefaultCostModel.cpp as it relies on cost_model.h.
# Skip GenGen.cpp and RunGenMain.cpp as they bring clang-tidy to its knees,
# for reasons that aren't entirely clear yet.
echo Finding targets...
CLANG_TIDY_TARGETS=$(find \
"${ROOT_DIR}/src" \
"${ROOT_DIR}/python_bindings" \
Expand All @@ -71,9 +95,10 @@ CLANG_TIDY_TARGETS=$(find \
# so we will instead build an include filter
CLANG_TIDY_HEADER_FILTER=".*/src/.*|.*/python_bindings/.*|.*/tools/.*|.*/util/.*"

echo Running clang-tidy...
${RUN_CLANG_TIDY} \
${FIX} \
-j $(nproc) \
-j ${J} \
-header-filter="${CLANG_TIDY_HEADER_FILTER}" \
-quiet \
-p ${CLANG_TIDY_BUILD_DIR} \
Expand Down
2 changes: 1 addition & 1 deletion src/Generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ int generate_filter_main_inner(int argc,
char **argv,
const GeneratorFactoryProvider &generator_factory_provider) {
static const char kUsage[] = R"INLINE_CODE(
gengen
gengenz
[-g GENERATOR_NAME] [-f FUNCTION_NAME] [-o OUTPUT_DIR] [-r RUNTIME_NAME]
[-d 1|0] [-e EMIT_OPTIONS] [-n FILE_BASE_NAME] [-p PLUGIN_NAME]
[-s AUTOSCHEDULER_NAME] [-t TIMEOUT]
Expand Down

0 comments on commit bda9e9a

Please sign in to comment.