Skip to content

Commit

Permalink
Merge 'build: use DPDK_MACHINE=haswell when testing dpdk build on git…
Browse files Browse the repository at this point in the history
…hub-hosted runner' from Kefu Chai

when building dpdk on a building host which has AVX10.1 ISA support,
with Clang 18, we have following failure
```
/usr/lib/llvm-18/bin/clang -Ilib/net/libnet_crc_avx512_lib.a.p -Ilib/net -I../../../../../../dpdk/lib/net -I. -I../../../../../../dpdk -Iconfig -I../../../../../../dpdk/config -Ilib/eal/include -I../../../../../../dpdk/lib/eal/include -Ilib/eal/linux/include -I../../../../../../dpdk/lib/eal/linux/include -Ilib/eal/x86/include -I../../../../../../dpdk/lib/eal/x86/include -Ilib/eal/common -I../../../../../../dpdk/lib/eal/common -Ilib/eal -I../../../../../../dpdk/lib/eal -Ilib/kvargs -I../../../../../../dpdk/lib/kvargs -Ilib/metrics -I../../../../../../dpdk/lib/metrics -Ilib/telemetry -I../../../../../../dpdk/lib/telemetry -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -O3 -include rte_config.h -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-packed-member -Wno-missing-field-initializers -D_GNU_SOURCE -Wno-error -fPIC -march=native -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation -DCC_X86_64_SSE42_PCLMULQDQ_SUPPORT -DCC_X86_64_AVX512_VPCLMULQDQ_SUPPORT -mavx512f -mavx512bw -mavx512dq -mavx512vl -mvpclmulqdq -mavx2 -mavx -MD -MQ lib/net/libnet_crc_avx512_lib.a.p/net_crc_avx512.c.o -MF lib/net/libnet_crc_avx512_lib.a.p/net_crc_avx512.c.o.d -o lib/net/libnet_crc_avx512_lib.a.p/net_crc_avx512.c.o -c ../../../../../../dpdk/lib/net/net_crc_avx512.c
Error: ../../../../../../dpdk/lib/net/net_crc_avx512.c:324:22: error: always_inline function '_mm512_broadcast_i32x4' requires target feature 'evex512', but would be inlined into function 'crc32_load_init_constants' that is compiled without support for 'evex512'
  324 |         crc32_eth.rk1_rk2 = _mm512_broadcast_i32x4(a);
      |                             ^
```

according to https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html
and
https://github.com/llvm/llvm-project/blob/release/18.x/clang/docs/UsersManual.rst#x86,
we should either use `-mevex512` or use `-mavx10.1-512` for accessing these new vectorized instructions provided by AVX10.1.

we also pass `-march=native` to the compiler. and it turns out that at least some github-hosted runners do not support AVX10.1 ISA, but the Clang 18 compiler does. so this combination breaks the build, as dpdk tries to check if compiler supports AVX512, and if it does, dpdk builds the AVX512 C files with this unsupported combination. that's why we
have build failures recently.

to address this issue, instead of using "native" as the value of `Seastar_DPDK_MACHINE`, let's use a more conservative but still capable architecture supported by github-hosted runner: "haswell", which supports the instruction set that is used by quite a few DPDK optimizations. so we can still have a decent coverage for building with DPDK, and for testing the build. please note, we don't test Seastar with the DPDK backend yet.

to enable the workflow to pass extra options to `configure.py`, we change the "cooks" parameter in the matrix to "options", and append "--dpdk-machine haswell" to it, so that we use this option only when testing the dpdk build.

Fixes scylladb#2242

Closes scylladb#2243

* github.com:scylladb/seastar:
  build: use DPDK_MACHINE=haswell when testing dpdk build on github-hosted runner
  build: add --dpdk-machine option to configure.py
  build: stop translating -march option to names recognized by DPDK
  • Loading branch information
avikivity committed May 16, 2024
2 parents 223eccf + ba395a2 commit 97aa247
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 30 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ jobs:
- compiler: clang++-18
standard: 23
mode: release
cooks: --cook dpdk
options: --cook dpdk --dpdk-machine haswell
enables: --enable-dpdk
- compiler: clang++-18
standard: 23
Expand All @@ -69,7 +69,7 @@ jobs:
steps:
- uses: actions/checkout@v4
with:
submodules: "${{ contains(matrix.cooks, 'dpdk') }}"
submodules: "${{ contains(matrix.enables, 'dpdk') }}"

- name: Install build dependencies
run: |
Expand Down Expand Up @@ -98,7 +98,7 @@ jobs:
--compiler $CXX
--c-compiler $CC
--mode ${{ matrix.mode }}
${{ matrix.cooks }}
${{ matrix.options }}
${{ matrix.enables }} ;
- name: Build
Expand Down
29 changes: 2 additions & 27 deletions configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def standard_supported(standard, compiler='g++'):
arg_parser.add_argument('--compile-commands-json', dest='cc_json', action='store_true',
help='Generate a compile_commands.json file for integration with clangd and other tools.')
arg_parser.add_argument('--heap-profiling', dest='heap_profiling', action='store_true', default=False, help='Enable heap profiling')
arg_parser.add_argument('--dpdk-machine', default='native', help='Specify the target architecture')
add_tristate(arg_parser, name='deferred-action-require-noexcept', dest='deferred_action_require_noexcept', help='noexcept requirement for deferred actions', default=True)
arg_parser.add_argument('--prefix', dest='install_prefix', default='/usr/local', help='Root installation path of Seastar files')
args = arg_parser.parse_args()
Expand All @@ -163,32 +164,6 @@ def identify_best_standard(cpp_standards, compiler):
args.cpp_standard = identify_best_standard(cpp_standards, compiler=args.cxx)


def infer_dpdk_machine(user_cflags):
"""Infer the DPDK machine identifier (e.g., 'ivb') from the space-separated
string of user cflags by scraping the value of `-march` if it is present.
The default if no architecture is indicated is 'native'.
"""
arch = 'native'

# `-march` may be repeated, and we want the last one.
# strip features, leave only the arch: armv8-a+crc+crypto -> armv8-a
for flag in user_cflags.split():
if flag.startswith('-march'):
arch = flag[7:].split('+')[0]

MAPPING = {
'native': 'native',
'nehalem': 'nhm',
'westmere': 'wsm',
'sandybridge': 'snb',
'ivybridge': 'ivb',
'armv8-a': 'armv8a',
}

return MAPPING.get(arch, 'native')


MODES = seastar_cmake.SUPPORTED_MODES if args.mode == 'all' else [args.mode]

# For convenience.
Expand Down Expand Up @@ -223,7 +198,7 @@ def configure_mode(mode):
tr(LDFLAGS, 'LD_FLAGS'),
tr(args.cxx_modules, 'MODULE'),
tr(args.dpdk, 'DPDK'),
tr(infer_dpdk_machine(args.user_cflags), 'DPDK_MACHINE'),
tr(args.dpdk_machine, 'DPDK_MACHINE'),
tr(args.hwloc, 'HWLOC', value_when_none='yes'),
tr(args.io_uring, 'IO_URING', value_when_none=None),
tr(args.alloc_failure_injection, 'ALLOC_FAILURE_INJECTION', value_when_none='DEFAULT'),
Expand Down

0 comments on commit 97aa247

Please sign in to comment.