Re: [dpdk-dev] [PATCH v5] eal_common_cpuflags: Fix %rbx corruption, and simplify the code

[Neil Horman <nhorman@tuxdriver.com>]

On Wed, Apr 16, 2014 at 08:58:31AM +0000, De Lara Guarch, Pablo wrote:
> Hi Neil,
> 
> 
> 
> >> What was the value of RTE_COMPILE_TIME_CPUFLAGS when you built your library?  If you added defines for flag support that are not tested for yet in the cpu_feature_table, that would be the error you would see I think
> 
> 
> 
> >RTE_COMPILE_TIME_CPUFLAGS= >RTE_CPUFLAG_SSE,RTE_CPUFLAG_SSE2,RTE_CPUFLAG_SSE3,RTE_CPUFLAG_SSSE3,RTE_CPUFLAG_SSE4_1,RTE_CPUFLAG_SSE4_2,RTE_CPUFLAG_AES,RTE_CPUFLAG_PCLMULQDQ,RTE_CPUFLAG_AVX
> 
> 
> 
> >For gcc_version >=4.6, that value is the same, of course (and all those flags are supported). So it has to be something else...
> 
> 
> 
> Silly me, I just saw what was hapenning. There are two missing brackets in the loop:
> 
> 
> 
> for (i = 0; i < sizeof(compile_time_flags)/sizeof(compile_time_flags[0]); i++) {
> 
> ret = rte_cpu_get_flag_enabled(compile_time_flags[i]);
> 
> 
> 
> if (ret < 0) {
> 
> fprintf(stderr,
> 
>   "ERROR: CPU feature flag lookup failed with error %d\n",
> 
>   ret);
> 
> exit(1);
> 
> }
> 
> if (!ret) {
> 
> fprintf(stderr,
> 
>   "ERROR: This system does not support \"%s\".\n"
> 
>   "Please check that RTE_MACHINE is set correctly.\n",
> 
>   cpu_feature_table[compile_time_flags[i]].name);
> 
> exit(1);
> 
> }
> 
> }
> 
> 
> 
> So, basically, "i" was size of the array +1 after the loop and only the return value was being checked for the last item. Not sure why higher versions of gcc do not catch this.
> 
Oh wow, yes, the if conditionals definately should be checked for each iteration
of the for loop. Good eye.

Still though, seems like a bug in gcc to check the state of the loop index on
exit, when its never used to index the array at that value.  Seems a bit like
this bug:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45180

Thomas I'll send a patch to fix this directly
Neil

> 
> 
> Thanks,
> 
> 
> 
> Pablo de Lara
> 
> DPDK SW Engineer
> 
> 
> 
> --------------------------------------------------------------
> 
> Intel Shannon Limited
> 
> Registered in Ireland
> 
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare
> 
> 
> 
> 
> 
> 
>