diff mbox series

[ovs-dev] sparse: Add immintrin.h header.

Message ID 20240430143656.563607-1-i.maximets@ovn.org
State Accepted
Commit bd8e9f48f180800292c10e12f26824833f18506a
Headers show
Series [ovs-dev] sparse: Add immintrin.h header. | expand

Checks

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

Commit Message

Ilya Maximets April 30, 2024, 2:36 p.m. UTC
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

Comments

Ales Musil April 30, 2024, 2:41 p.m. UTC | #1
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>
Ilya Maximets April 30, 2024, 7:03 p.m. UTC | #2
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 mbox series

Patch

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>