Skip to content

Commit

Permalink
chore(build): replace -Wc++-compat' with -x c++'
Browse files Browse the repository at this point in the history
As of this commit, a build with CFLAGS="-x c++" succeeds; ie: all source
files in the tree are both valid C++ source code AND valid C code.

Convert wcpp-compat distcheck builds to actually build with C++, to
prevent future pull requests from breaking C++ compatibility going
forward.

stack-info: PR: #578, branch: aws-nslick/stack/25
Signed-off-by: Nicholas Sielicki <nslick@amazon.com>
  • Loading branch information
aws-nslick committed Sep 14, 2024
1 parent 5d80033 commit e537a8d
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 8 deletions.
30 changes: 24 additions & 6 deletions .github/workflows/distcheck.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ jobs:
matrix:
sdk:
- cuda
mode:
- cpp
- c
amazonlinux:
- al2023
- al2
Expand All @@ -47,7 +50,7 @@ jobs:
cudapackages: cuda-cudart-devel-12-3 cuda-driver-devel-12-3

runs-on: codebuild-ghactions-${{ matrix.amazonlinux }}-${{ github.run_id }}-${{ github.run_attempt }}
name: ${{matrix.amazonlinux}}/${{ matrix.sdk }}/efa@${{ matrix.efainstaller }}/makeinstall
name: ${{matrix.amazonlinux}}/${{ matrix.sdk }}/efa@${{ matrix.efainstaller }}/makeinstall(${{ matrix.mode }})
steps:
# note, do not bump to v4: https://github.com/actions/checkout/issues/1590
- uses: actions/checkout@v3
Expand Down Expand Up @@ -76,6 +79,7 @@ jobs:
./configure --prefix=/opt/aws-ofi-nccl --with-mpi=/opt/amazon/openmpi \
--with-libfabric=/opt/amazon/efa \
--with-cuda=/usr/local/cuda \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-tests=no \
--enable-platform-aws
Expand All @@ -95,13 +99,15 @@ jobs:
cc:
- gcc
- clang
mode:
- cpp
- c
tracing:
- lttng
- none
sdk:
- cuda
- neuron

include:
- cc-variant: latest
cc: clang
Expand All @@ -110,7 +116,7 @@ jobs:
cc: gcc
cc-version: 13

name: u2204/${{ matrix.sdk }}/libfabric@git/${{matrix.cc}}(${{matrix.cc-variant}})/distcheck/
name: u2204/${{ matrix.sdk }}/libfabric@git/${{matrix.cc}}(${{matrix.cc-variant}})/distcheck(${{ matrix.mode }})/
steps:
- uses: actions/checkout@v4
- name: Configure Neuron SDK Repository
Expand Down Expand Up @@ -196,10 +202,12 @@ jobs:
./configure --with-mpi=/opt/amazon/openmpi \
--with-libfabric=/opt/amazon/efa \
--with-cuda=/usr/local/cuda/ \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-tests \
--enable-platform-aws
else
./configure --with-libfabric=/opt/amazon/efa \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-neuron \
--enable-platform-aws
fi
Expand All @@ -226,6 +234,9 @@ jobs:
cc:
- gcc
- clang
mode:
- cpp
- c
sdk:
- cuda
- neuron
Expand All @@ -237,7 +248,7 @@ jobs:
cc: gcc
cc-version: 13

name: u2204/${{ matrix.sdk }}/${{matrix.cc}}(${{matrix.cc-variant}})/unit-tests/
name: u2204/${{ matrix.sdk }}/${{matrix.cc}}(${{matrix.cc-variant}})/unit-tests(${{ matrix.mode }})/
steps:
- uses: actions/checkout@v4
- name: Configure Neuron SDK Repository
Expand Down Expand Up @@ -316,13 +327,15 @@ jobs:
./configure --with-mpi=/opt/amazon/openmpi \
--with-libfabric=/opt/amazon/efa \
--with-cuda=/usr/local/cuda/ \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-tests \
--enable-debug \
--enable-platform-aws
else
./configure --with-libfabric=/opt/amazon/efa \
--enable-neuron \
--enable-debug \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-platform-aws
fi
make -j "$(nproc)"
Expand All @@ -346,7 +359,10 @@ jobs:
sdk:
- cuda
- neuron
name: CodeChecker - ${{ matrix.sdk }}
mode:
- cpp
- c
name: CodeChecker - ${{ matrix.sdk }} ${{ matrix.mode }}
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
Expand Down Expand Up @@ -404,12 +420,14 @@ jobs:
./configure \
--with-libfabric="/opt/amazon/efa" \
--enable-neuron \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-platform-aws
else
./configure \
--with-libfabric="/opt/amazon/efa" \
--with-mpi="/opt/amazon/openmpi" \
--with-cuda=/usr/local/cuda/ \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-tests \
--enable-platform-aws
fi
Expand All @@ -428,7 +446,7 @@ jobs:
- name: Save CodeChecker HTML output.
uses: actions/upload-artifact@v4
with:
name: CodeChecker Bug Reports for ${{ matrix.sdk }}
name: CodeChecker Bug Reports for ${{ matrix.sdk }} ${{ matrix.mode }}
path: ${{ steps.codechecker.outputs.result-html-dir }}/*.html

- name: CodeChecker Pass Or Fail?
Expand Down
10 changes: 8 additions & 2 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,13 @@ AC_DEFINE_UNQUOTED([OFI_NCCL_TRACE], [${trace}], [Defined to 1 unit test output
AC_ARG_ENABLE([picky-compiler],
[AS_HELP_STRING([--disable-picky-compiler], [Disable adding picky compiler flags.])])
AS_IF([test "${enable_picky_compiler}" != "no"],
[picky_compiler_flags="-Wall -Wc++-compat -Wextra -Wno-unused-parameter"
[picky_compiler_flags="-Wall -Wextra -Wno-unused-parameter"
AC_MSG_NOTICE([Adding ${picky_compiler_flags} to CFLAGS.])
CFLAGS="${CFLAGS} ${picky_compiler_flags}"
AS_UNSET([picky_compiler_flags])])

AC_ARG_ENABLE([werror],
[AS_HELP_STRING([--enable-werror], [Enable setting -Werror. Off by default, unless building from Git tree.])])
[AS_HELP_STRING([--enable-werror], [(Developer) Enable setting -Werror. Off by default, unless building from Git tree.])])
werror_flags="-Werror -Wno-error=unused-parameter"
AS_IF([test -d "${srcdir}/.git" -a -z "${enable_werror}"],
[AC_MSG_NOTICE([Found .git directory. Adding ${werror_flags} to CFLAGS.])
Expand All @@ -159,6 +159,12 @@ AS_IF([test -d "${srcdir}/.git" -a -z "${enable_werror}"],
[AC_MSG_NOTICE([Adding ${werror_flags} to CFLAGS.])
CFLAGS="${CFLAGS} ${werror_flags}"])

AC_ARG_ENABLE([cpp],
[AS_HELP_STRING([--enable-cpp], [(Developer) Enable building as C++ source. Off by default.])])
AS_IF([test "${enable_cpp}" = "yes"],
[AC_MSG_NOTICE([Building as C++ source.])
CFLAGS="${CFLAGS} -x c++ -std=c++17"])

AC_CONFIG_FILES([Makefile
include/Makefile
src/Makefile
Expand Down

0 comments on commit e537a8d

Please sign in to comment.