Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: use DPDK_MACHINE=haswell when testing dpdk build on github-hosted runner #2243

Merged
merged 3 commits into from
May 16, 2024

Conversation

tchaikov
Copy link
Contributor

@tchaikov tchaikov commented May 16, 2024

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 #2242

@tchaikov tchaikov force-pushed the dpdk-build-avx10 branch 5 times, most recently from 06eed8e to b029875 Compare May 16, 2024 02:00
tchaikov added 2 commits May 16, 2024 10:22
we dropped the translation in 369d166, when upgrading to DPDK
v23.07, as newer DPDK recognizes these names respected by `-march`,
so there is no point to preserve this translation in `configure.py`
anymore.

in this change, the helper of `infer_dpdk_machine()` along with
its caller are dropped.

Signed-off-by: Kefu Chai <[email protected]>
this allows user and our toolings to specify the target architecture
when building Seastar. in particular, it prepares us to avoid using
the default "-march=native" setting when building Seastar with DPDK
enabled, which could fail the build when building with a compiler
supporting AVX512, while the building host does not.

Signed-off-by: Kefu Chai <[email protected]>
@tchaikov tchaikov force-pushed the dpdk-build-avx10 branch 2 times, most recently from d6512ed to 98b4090 Compare May 16, 2024 02:28
…ted runner

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. but we also
pass "-march=native" to the compiler. 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

Signed-off-by: Kefu Chai <[email protected]>
@tchaikov tchaikov force-pushed the dpdk-build-avx10 branch from 98b4090 to ba395a2 Compare May 16, 2024 02:30
@tchaikov tchaikov changed the title build: apply patch when building dpdk to use evex512 ISA build: use DPDK_MACHINE=haswell when testing dpdk build on github-hosted runner May 16, 2024
@tchaikov tchaikov added the build build system label May 16, 2024
@tchaikov

This comment was marked as outdated.

@avikivity avikivity merged commit 97aa247 into scylladb:master May 16, 2024
26 checks passed
@tchaikov tchaikov deleted the dpdk-build-avx10 branch May 16, 2024 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

the workflow of "Tests (clang++-18, C++23, release, --enable-dpdk) fails due to dpdk build failure
2 participants