> -----Original Message----- > From: Neil Horman [mailto:nhor...@tuxdriver.com] > Sent: Thursday, August 10, 2017 12:34 PM > To: Richardson, Bruce <bruce.richard...@intel.com> > Cc: dev@dpdk.org; Thomas Monjalon <tho...@monjalon.net> > Subject: Re: [dpdk-dev] [PATCH] atm: Remove machine definition > > On Thu, Aug 10, 2017 at 09:34:18AM +0100, Bruce Richardson wrote: > > On Wed, Aug 09, 2017 at 04:24:25PM -0400, Neil Horman wrote: > > > With the new updated requirement for SSE4.2, dpdk no longer supports > > > building on atom machines, as they only support up to SSE3. Remove > > > the machine definition. > > > > > > Signed-off-by: Neil Horman <nhor...@tuxdriver.com> CC: Thomas > > > Monjalon <tho...@monjalon.net> --- mk/machine/atm/rte.vars.mk | 58 > > > ---------------------------------------------- 1 file changed, 58 > > > deletions(-) delete mode 100644 mk/machine/atm/rte.vars.mk > > > > > Yes, good catch, that should have been removed. However, I think the > > commit log should be updated to mention that it no longer supports > > "early" atom machines, or some similar phrase. Atom cores for the last > > number of years do support SSE4, see: > > https://en.wikipedia.org/wiki/Silvermont for example. > > > > I don't really agree with that. If you want to make the claim that only > early atom machines aren't supported, the implication then is that later > atom machines are, and we should keep the machine type, and fix it to > build for those targeted machines (as it stands currently, the compiler > errors out on building atom because it only emits SSE3 instructions and > can't inline an SSE4 builtin). I'd be totally fine with fixing the atom > build (which I imageine amounts to passing -machine=atom -msse4 or > something simmilar to the build options. Once we do that however, we > likely need a runtime check for sse4 support as well, so we don't just get > random SIGILL errors at run time. > > Neil
For the runtime checks, "rte_eal_init()" already calls "rte_cpu_is_supported()" checks to ensure that all instruction sets that are built in are supported by the runtime platform. For fixing the atom build, I actually thought we were moving away from having special confirm files for the rte_machine type, and just passing the RTE_MACHINE build-time value directly through to the compiler as -march - especially since gcc and clang have started using the microarchitecture names directly in the march flag rather than the older names like core-i7-avx. In fact, "atom" is no longer listed in the manpage for gcc on my Fedora 26 system, only "bonnell", "silvermont" are present as atom architectures. If you see a need for fixing the atm build file, I have no objections, but I think removing it is an acceptable solution as anyone who needs it can build for atom by manually editing the RTE_MACHINE type to "silvermont". [Though, I do think the comments in config/common_base could do with being updated, since it's not required any more to have a build directory for each machine type.] Regards, /Bruce
