Message ID | 20240430143656.563607-1-i.maximets@ovn.org |
---|---|
State | Accepted |
Commit | bd8e9f48f180800292c10e12f26824833f18506a |
Headers | show |
Series | [ovs-dev] sparse: Add immintrin.h header. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
ovsrobot/intel-ovs-compilation | success | test: success |
On Tue, Apr 30, 2024 at 4:36 PM Ilya Maximets <i.maximets@ovn.org> wrote: > Sparse doesn't understand _Float16 and some other types used by > immintrin.h from GCC 13. This breaks sparse builds with DPDK on > Fedora 38+ and Ubuntu 24.04. > > Add another sparse-specific header to workaround the problem. We do > need some of the functions and types defined in these headers, so we > can't really stab out the whole header. Carving out the main offenders > instead by defining the inclusion guards. > > This is fragile and depends on internals of immintrin and underlying > headers, but I'm not sure what the better way to solve the issue > would be. This approach should be more or less portable between > compilers, because it only defines a few specific variables. We may > have to add more as GCC headers change over time. > > This fixes the build with a following config on F38 and Ubuntu 24.04: > > ./configure --enable-sparse --with-dpdk=yes --enable-Werror > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > include/sparse/automake.mk | 1 + > include/sparse/immintrin.h | 30 ++++++++++++++++++++++++++++++ > 2 files changed, 31 insertions(+) > create mode 100644 include/sparse/immintrin.h > > diff --git a/include/sparse/automake.mk b/include/sparse/automake.mk > index c1229870b..45e6202c5 100644 > --- a/include/sparse/automake.mk > +++ b/include/sparse/automake.mk > @@ -1,5 +1,6 @@ > noinst_HEADERS += \ > include/sparse/rte_byteorder.h \ > + include/sparse/immintrin.h \ > include/sparse/xmmintrin.h \ > include/sparse/arpa/inet.h \ > include/sparse/bits/floatn.h \ > diff --git a/include/sparse/immintrin.h b/include/sparse/immintrin.h > new file mode 100644 > index 000000000..dd742be9f > --- /dev/null > +++ b/include/sparse/immintrin.h > @@ -0,0 +1,30 @@ > +/* Copyright (c) 2024 Red Hat, Inc. > + * > + * 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. > + */ > + > +#ifndef __CHECKER__ > +#error "Use this header only with sparse. It is not a correct > implementation." > +#endif > + > +/* Sparse doesn't know some types used by AVX512 and some other headers. > + * Mark those headers as already included to avoid failures. This is > fragile, > + * so may need adjustments with compiler changes. */ > +#define _AVX512BF16INTRIN_H_INCLUDED > +#define _AVX512BF16VLINTRIN_H_INCLUDED > +#define _AVXNECONVERTINTRIN_H_INCLUDED > +#define _KEYLOCKERINTRIN_H_INCLUDED > +#define __AVX512FP16INTRIN_H_INCLUDED > +#define __AVX512FP16VLINTRIN_H_INCLUDED > + > +#include_next <immintrin.h> > -- > 2.44.0 > > Looks good to me, thanks! Acked-by: Ales Musil <amusil@redhat.com>
On 4/30/24 16:41, Ales Musil wrote: > > > On Tue, Apr 30, 2024 at 4:36 PM Ilya Maximets <i.maximets@ovn.org <mailto:i.maximets@ovn.org>> wrote: > > Sparse doesn't understand _Float16 and some other types used by > immintrin.h from GCC 13. This breaks sparse builds with DPDK on > Fedora 38+ and Ubuntu 24.04. > > Add another sparse-specific header to workaround the problem. We do > need some of the functions and types defined in these headers, so we > can't really stab out the whole header. Carving out the main offenders > instead by defining the inclusion guards. > > This is fragile and depends on internals of immintrin and underlying > headers, but I'm not sure what the better way to solve the issue > would be. This approach should be more or less portable between > compilers, because it only defines a few specific variables. We may > have to add more as GCC headers change over time. > > This fixes the build with a following config on F38 and Ubuntu 24.04: > > ./configure --enable-sparse --with-dpdk=yes --enable-Werror > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org <mailto:i.maximets@ovn.org>> > --- > > Looks good to me, thanks! > > Acked-by: Ales Musil <amusil@redhat.com <mailto:amusil@redhat.com>> Thanks! Applied to all branches down to 2.17. Best regards, Ilya Maximets.
diff --git a/include/sparse/automake.mk b/include/sparse/automake.mk index c1229870b..45e6202c5 100644 --- a/include/sparse/automake.mk +++ b/include/sparse/automake.mk @@ -1,5 +1,6 @@ noinst_HEADERS += \ include/sparse/rte_byteorder.h \ + include/sparse/immintrin.h \ include/sparse/xmmintrin.h \ include/sparse/arpa/inet.h \ include/sparse/bits/floatn.h \ diff --git a/include/sparse/immintrin.h b/include/sparse/immintrin.h new file mode 100644 index 000000000..dd742be9f --- /dev/null +++ b/include/sparse/immintrin.h @@ -0,0 +1,30 @@ +/* Copyright (c) 2024 Red Hat, Inc. + * + * 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. + */ + +#ifndef __CHECKER__ +#error "Use this header only with sparse. It is not a correct implementation." +#endif + +/* Sparse doesn't know some types used by AVX512 and some other headers. + * Mark those headers as already included to avoid failures. This is fragile, + * so may need adjustments with compiler changes. */ +#define _AVX512BF16INTRIN_H_INCLUDED +#define _AVX512BF16VLINTRIN_H_INCLUDED +#define _AVXNECONVERTINTRIN_H_INCLUDED +#define _KEYLOCKERINTRIN_H_INCLUDED +#define __AVX512FP16INTRIN_H_INCLUDED +#define __AVX512FP16VLINTRIN_H_INCLUDED + +#include_next <immintrin.h>
Sparse doesn't understand _Float16 and some other types used by immintrin.h from GCC 13. This breaks sparse builds with DPDK on Fedora 38+ and Ubuntu 24.04. Add another sparse-specific header to workaround the problem. We do need some of the functions and types defined in these headers, so we can't really stab out the whole header. Carving out the main offenders instead by defining the inclusion guards. This is fragile and depends on internals of immintrin and underlying headers, but I'm not sure what the better way to solve the issue would be. This approach should be more or less portable between compilers, because it only defines a few specific variables. We may have to add more as GCC headers change over time. This fixes the build with a following config on F38 and Ubuntu 24.04: ./configure --enable-sparse --with-dpdk=yes --enable-Werror Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- include/sparse/automake.mk | 1 + include/sparse/immintrin.h | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 include/sparse/immintrin.h