[dpdk-dev] mk: fix compiling error for atom target

Message ID 1504773317-2042-1-git-send-email-xiaoyun.li@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Li, Xiaoyun Sept. 7, 2017, 8:35 a.m. UTC
  GCC thinks target atom doesn't support SSE4.2 and SSE4 is now part of
minimum requirements for DPDK on x86. So there are compiling errors when
cross-compiling for target atom. And in fact, the atom supports SSE4 now.
This patch fixes this issue.

Fixes: 5ea4d4688dce ("net/ixgbe: remove fallback code for x86 non-SSE4")
Cc: stable@dpdk.org

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
 mk/machine/atm/rte.vars.mk | 2 ++
 1 file changed, 2 insertions(+)
  

Comments

Bruce Richardson Sept. 7, 2017, 8:56 a.m. UTC | #1
On Thu, Sep 07, 2017 at 04:35:17PM +0800, Xiaoyun Li wrote:
> GCC thinks target atom doesn't support SSE4.2 and SSE4 is now part of
> minimum requirements for DPDK on x86. So there are compiling errors when
> cross-compiling for target atom. And in fact, the atom supports SSE4 now.
> This patch fixes this issue.
> 
> Fixes: 5ea4d4688dce ("net/ixgbe: remove fallback code for x86 non-SSE4")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>

The -march=atom flag is for older atom CPUs, and is no longer advertised
by gcc, since all -march flags now correspond to the actual core
microarchitecture used. [man gcc on Fedora 26, no longer lists atom as a
valid march value, though it's probably still accepted for backward
compatibility]. The direct replacement for atom is "bonnell", which does
not have SSE4.2 support, so we should not use. What we do support is
later atoms, so the architecture targets for atom should be
"-march=silvermont". This automatically includes sse4 support so no
additional flags needed.

Note: I previously had this discussion with Neil on-list, but forgot to
do the follow-up work agreed on. See: 
https://www.mail-archive.com/dev@dpdk.org/msg72629.html
The agreed plan was to remove atom as a target for DPDK builds and add a
"silvermont" replacement instead. If you have time, perhaps you could
look at doing that instead of this fix?

/Bruce

> ---
>  mk/machine/atm/rte.vars.mk | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mk/machine/atm/rte.vars.mk b/mk/machine/atm/rte.vars.mk
> index cfed110..a6899d9 100644
> --- a/mk/machine/atm/rte.vars.mk
> +++ b/mk/machine/atm/rte.vars.mk
> @@ -56,3 +56,5 @@
>  # CPU_ASFLAGS =
>  
>  MACHINE_CFLAGS = -march=atom
> +
> +MACHINE_CFLAGS += -msse4.2
> -- 
> 2.7.4
>
  
Li, Xiaoyun Sept. 7, 2017, 9:31 a.m. UTC | #2
Hi
So I should delete the mk/machine/atm folder and add a new mk/machine/silvermont folder, right?
And in silvermont/rte.vars.mk, there is sentence " MACHINE_CFLAGS = -march=silvermont ".
The annotation just adds the copy of others like original atm?
Another thing, we don't need to create the config file (defconfig_x86_64-silvermont-linuxapp-gcc), right?

Best Regards
Xiaoyun Li



-----Original Message-----
From: Richardson, Bruce 
Sent: Thursday, September 7, 2017 16:57
To: Li, Xiaoyun <xiaoyun.li@intel.com>
Cc: dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>; stable@dpdk.org
Subject: Re: [PATCH] mk: fix compiling error for atom target

On Thu, Sep 07, 2017 at 04:35:17PM +0800, Xiaoyun Li wrote:
> GCC thinks target atom doesn't support SSE4.2 and SSE4 is now part of 
> minimum requirements for DPDK on x86. So there are compiling errors 
> when cross-compiling for target atom. And in fact, the atom supports SSE4 now.
> This patch fixes this issue.
> 
> Fixes: 5ea4d4688dce ("net/ixgbe: remove fallback code for x86 
> non-SSE4")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>

The -march=atom flag is for older atom CPUs, and is no longer advertised by gcc, since all -march flags now correspond to the actual core microarchitecture used. [man gcc on Fedora 26, no longer lists atom as a valid march value, though it's probably still accepted for backward compatibility]. The direct replacement for atom is "bonnell", which does not have SSE4.2 support, so we should not use. What we do support is later atoms, so the architecture targets for atom should be "-march=silvermont". This automatically includes sse4 support so no additional flags needed.

Note: I previously had this discussion with Neil on-list, but forgot to do the follow-up work agreed on. See: 
https://www.mail-archive.com/dev@dpdk.org/msg72629.html
The agreed plan was to remove atom as a target for DPDK builds and add a "silvermont" replacement instead. If you have time, perhaps you could look at doing that instead of this fix?

/Bruce

> ---
>  mk/machine/atm/rte.vars.mk | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mk/machine/atm/rte.vars.mk b/mk/machine/atm/rte.vars.mk 
> index cfed110..a6899d9 100644
> --- a/mk/machine/atm/rte.vars.mk
> +++ b/mk/machine/atm/rte.vars.mk
> @@ -56,3 +56,5 @@
>  # CPU_ASFLAGS =
>  
>  MACHINE_CFLAGS = -march=atom
> +
> +MACHINE_CFLAGS += -msse4.2
> --
> 2.7.4
>
  
Bruce Richardson Sept. 7, 2017, 10:09 a.m. UTC | #3
On Thu, Sep 07, 2017 at 10:31:17AM +0100, Li, Xiaoyun wrote:
> Hi
> So I should delete the mk/machine/atm folder and add a new mk/machine/silvermont folder, right?
> And in silvermont/rte.vars.mk, there is sentence " MACHINE_CFLAGS = -march=silvermont ".
> The annotation just adds the copy of others like original atm?
> Another thing, we don't need to create the config file (defconfig_x86_64-silvermont-linuxapp-gcc), right?
> 

Yes, that is correct.

> Best Regards
> Xiaoyun Li
>
Thanks,
/Bruce
> 
> 
> -----Original Message-----
> From: Richardson, Bruce 
> Sent: Thursday, September 7, 2017 16:57
> To: Li, Xiaoyun <xiaoyun.li@intel.com>
> Cc: dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>; stable@dpdk.org
> Subject: Re: [PATCH] mk: fix compiling error for atom target
> 
> On Thu, Sep 07, 2017 at 04:35:17PM +0800, Xiaoyun Li wrote:
> > GCC thinks target atom doesn't support SSE4.2 and SSE4 is now part of 
> > minimum requirements for DPDK on x86. So there are compiling errors 
> > when cross-compiling for target atom. And in fact, the atom supports SSE4 now.
> > This patch fixes this issue.
> > 
> > Fixes: 5ea4d4688dce ("net/ixgbe: remove fallback code for x86 
> > non-SSE4")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> 
> The -march=atom flag is for older atom CPUs, and is no longer advertised by gcc, since all -march flags now correspond to the actual core microarchitecture used. [man gcc on Fedora 26, no longer lists atom as a valid march value, though it's probably still accepted for backward compatibility]. The direct replacement for atom is "bonnell", which does not have SSE4.2 support, so we should not use. What we do support is later atoms, so the architecture targets for atom should be "-march=silvermont". This automatically includes sse4 support so no additional flags needed.
> 
> Note: I previously had this discussion with Neil on-list, but forgot to do the follow-up work agreed on. See: 
> https://www.mail-archive.com/dev@dpdk.org/msg72629.html
> The agreed plan was to remove atom as a target for DPDK builds and add a "silvermont" replacement instead. If you have time, perhaps you could look at doing that instead of this fix?
> 
> /Bruce
> 
> > ---
> >  mk/machine/atm/rte.vars.mk | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/mk/machine/atm/rte.vars.mk b/mk/machine/atm/rte.vars.mk 
> > index cfed110..a6899d9 100644
> > --- a/mk/machine/atm/rte.vars.mk
> > +++ b/mk/machine/atm/rte.vars.mk
> > @@ -56,3 +56,5 @@
> >  # CPU_ASFLAGS =
> >  
> >  MACHINE_CFLAGS = -march=atom
> > +
> > +MACHINE_CFLAGS += -msse4.2
> > --
> > 2.7.4
> >
  

Patch

diff --git a/mk/machine/atm/rte.vars.mk b/mk/machine/atm/rte.vars.mk
index cfed110..a6899d9 100644
--- a/mk/machine/atm/rte.vars.mk
+++ b/mk/machine/atm/rte.vars.mk
@@ -56,3 +56,5 @@ 
 # CPU_ASFLAGS =
 
 MACHINE_CFLAGS = -march=atom
+
+MACHINE_CFLAGS += -msse4.2