On Wed, Feb 05, 2025 at 05:32:11PM +0000, Bruce Richardson wrote:
> On Wed, Feb 05, 2025 at 06:03:16PM +0100, David Marchand wrote:
> > On Wed, Feb 5, 2025 at 5:19 PM Bruce Richardson
> > <bruce.richard...@intel.com> wrote:
> > >
> > > The Intel-produced compiler "icc" has been replaced by the newer
> > > clang-based "icx" compiler. DPDK compilation has also not been tested
> > > recently with the icc compiler, so let's remove doc and code references
> > > to icc, and any special macros or build support that was added for it.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richard...@intel.com>
> > 
> > I noticed remaining references, from which some can be cleaned up too:
> > 
> > app/test-pmd/testpmd.h: * Work-around of a compilation error with ICC
> > on invocations of the
> > 
> 
> Yes, I spotted this and a few of the others too - but forgot to reference
> them in the commit log ntoes.
> 
> The trouble is that for a number of these cases I've no idea what specific
> part of the code the workaround is, or what it should look like without the
> workaround. 
> 
> For this specific instance, the ifdefs in the testpmd.h file just refer to
> GCC and non-GCC (presumably including clang), so it doesn't seem that there
> is code that is ICC specific. Shall I just remove the comment?
> 
> > lib/eal/common/eal_common_dynmem.c:                     /* to prevent
> > icc errors */
> 
> Same here. It's not clear to me what way the code should be reworked if not
> supporting ICC. Again, I can just remove the comment and be done with it.
> 
> > lib/eal/x86/include/rte_vect.h:#if defined(__ICC) || defined(_WIN64)
> > 
> 
> This seems as miss on my part. Will fix in a v2.
> 
> > buildtools/check-symbols.sh:# Filter out symbols suffixed with a . for icc
> > buildtools/check-symbols.sh:# Filter out symbols suffixed with a . for icc
> > 
> 
> This you may be able to advise me on. I assume that the icc-specific bit is
> just he "&& !($NF ~ /\.$/)" bit, and not the whole block the comment is put
> on?
> 

There is also a reference in ena driver, again, not sure what the correct
code replacement is - setting the struct initializer to {0} perhaps? Adding
some ena maintainers on CC so they can comment on this.

        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 };

Ena maintainers - any suggestions on what the code should look like if we
no longer have to support the icc compiler?

/Bruce

Reply via email to