Re: [PATCH v2 2/2] build: replace support for icc with icx

[David Marchand <david.marchand@redhat.com>]

On Wed, Feb 5, 2025 at 7:29 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 260e4761bd..9e6a7d553a 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -1292,10 +1292,6 @@ RTE_INIT(__##c) \
>         testpmd_add_driver_commands(&c); \
>  }
>
> -/*
> - * Work-around of a compilation error with ICC on invocations of the
> - * rte_be_to_cpu_16() function.
> - */
>  #ifdef __GCC__
>  #define RTE_BE_TO_CPU_16(be_16_v)  rte_be_to_cpu_16((be_16_v))
>  #define RTE_CPU_TO_BE_16(cpu_16_v) rte_cpu_to_be_16((cpu_16_v))

You may entirely drop those RTE_BE_TO_CPU_16 macros and just rely on
rte_be_to_cpu_16.


> diff --git a/buildtools/check-symbols.sh b/buildtools/check-symbols.sh
> index e458c0af72..b8ac24391e 100755
> --- a/buildtools/check-symbols.sh
> +++ b/buildtools/check-symbols.sh
> @@ -45,9 +45,8 @@ do
>         fi
>  done
>
> -# Filter out symbols suffixed with a . for icc
>  for SYM in `awk '{
> -       if ($2 != "l" && $4 == ".text.experimental" && !($NF ~ /\.$/)) {
> +       if ($2 != "l" && $4 == ".text.experimental") {
>                 print $NF
>         }
>  }' $DUMPFILE`
> @@ -76,9 +75,8 @@ do
>         fi
>  done
>
> -# Filter out symbols suffixed with a . for icc
>  for SYM in `awk '{
> -       if ($2 != "l" && $4 == ".text.internal" && !($NF ~ /\.$/)) {
> +       if ($2 != "l" && $4 == ".text.internal") {
>                 print $NF
>         }
>  }' $DUMPFILE`

LGTM, this is what I had in mind too.

>  API Changes
>  -----------
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index aea2e5c929..4c6af8285e 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -1413,7 +1413,6 @@ static int ena_create_io_queue(struct rte_eth_dev *dev, struct ena_ring *ring)
>         struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>         struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
>         struct ena_com_create_io_ctx ctx =
> -               /* policy set to _HOST just to satisfy icc compiler */
>                 { ENA_ADMIN_PLACEMENT_POLICY_HOST,
>                   0, 0, 0, 0, 0 };
>         uint16_t ena_qid;

No clue what this was about..

Looking at git history, it looks like something that got copy/pasted
over time...
I guess we can simply drop it.


> diff --git a/lib/eal/common/eal_common_dynmem.c b/lib/eal/common/eal_common_dynmem.c
> index b4dc231940..3c34a7defb 100644
> --- a/lib/eal/common/eal_common_dynmem.c
> +++ b/lib/eal/common/eal_common_dynmem.c
> @@ -510,7 +510,6 @@ eal_dynmem_calc_num_pages_per_socket(
>                 /* if we didn't satisfy all memory requirements per socket */
>                 if (memory[socket] > 0 &&
>                                 internal_conf->socket_mem[socket] != 0) {
> -                       /* to prevent icc errors */
>                         requested = (unsigned int)(
>                                 internal_conf->socket_mem[socket] / 0x100000);

I suspect those casts on socket_mem[] are unnecessary (and later, in
the same file).

>                         available = requested -


-- 
David Marchand