Re: [PATCH 14/15] app/test: turn off cpu flag checks for tile architecture

[Tony Lu <zlu-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org>]

>-----Original Message-----
>From: dev [mailto:dev-bounces-VfR2kkLFssw@public.gmane.org] On Behalf Of Neil Horman
>Sent: Tuesday, December 09, 2014 11:03 PM
>To: Zhigang Lu
>Cc: dev-VfR2kkLFssw@public.gmane.org
>Subject: Re: [dpdk-dev] [PATCH 14/15] app/test: turn off cpu flag checks
for tile
>architecture
>
>On Mon, Dec 08, 2014 at 04:59:37PM +0800, Zhigang Lu wrote:
>> Tile processor doesn't have CPU flag hardware registers, so this patch
>> turns off cpu flag checks for tile.
>>
>> Signed-off-by: Zhigang Lu <zlu-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Cyril Chemparathy <cchemparathy-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  app/test/test_cpuflags.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/app/test/test_cpuflags.c b/app/test/test_cpuflags.c index
>> 5aeba5d..da93af5 100644
>> --- a/app/test/test_cpuflags.c
>> +++ b/app/test/test_cpuflags.c
>> @@ -113,7 +113,7 @@ test_cpuflags(void)
>>
>>  	printf("Check for ICACHE_SNOOP:\t\t");
>>  	CHECK_FOR_FLAG(RTE_CPUFLAG_ICACHE_SNOOP);
>> -#else
>> +#elif !defined(RTE_ARCH_TILE)
>>  	printf("Check for SSE:\t\t");
>>  	CHECK_FOR_FLAG(RTE_CPUFLAG_SSE);
>>
>Please stop this.  It doesn't make sense for a library that supports
multiple
>arches, we need some way to generically test for flags that doesn't involve
>forcing applications to do ton's of ifdeffing.  Perhaps
rte_cpu_get_flag_enabled
>needs to do a flag table lookup based on the detected arch at run time, and
>return the appropriate response.  In the case of tile, it can just be an
empty
>table, so 0 is always returned.  But making an application responsible for
doing
>arch checks is a guarantee to write non-portable applications
>
>Neil
>

Thanks for taking a look at this.
This change just follows what PPC did in commit 9ae15538. The root cause is
that
the test_cpuflags.c explicitly tests X86-specific CPU flags, so we might
need to revise
this test case to make it architecture-independent.

A alternative change to this test case is as follows.
--- a/app/test/test_cpuflags.c
+++ b/app/test/test_cpuflags.c
@@ -77,81 +77,13 @@ cpu_flag_result(int result)
 static int
 test_cpuflags(void)
 {
-       int result;
+       int i, result;
        printf("\nChecking for flags from different registers...\n");
 
-#ifdef RTE_ARCH_PPC_64
-       printf("Check for PPC64:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_PPC64);
-
-       printf("Check for PPC32:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_PPC32);
-
-       printf("Check for VSX:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_VSX);
-
-       printf("Check for DFP:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_DFP);
-
-       printf("Check for FPU:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_FPU);
-
-       printf("Check for SMT:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_SMT);
-
-       printf("Check for MMU:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_MMU);
-
-       printf("Check for ALTIVEC:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_ALTIVEC);
-
-       printf("Check for ARCH_2_06:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_ARCH_2_06);
-
-       printf("Check for ARCH_2_07:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_ARCH_2_07);
-
-       printf("Check for ICACHE_SNOOP:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_ICACHE_SNOOP);
-#else
-       printf("Check for SSE:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_SSE);
-
-       printf("Check for SSE2:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_SSE2);
-
-       printf("Check for SSE3:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_SSE3);
-
-       printf("Check for SSE4.1:\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_SSE4_1);
-
-       printf("Check for SSE4.2:\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_SSE4_2);
-
-       printf("Check for AVX:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_AVX);
-
-       printf("Check for AVX2:\t\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_AVX2);
-
-       printf("Check for TRBOBST:\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_TRBOBST);
-
-       printf("Check for ENERGY_EFF:\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_ENERGY_EFF);
-
-       printf("Check for LAHF_SAHF:\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_LAHF_SAHF);
-
-       printf("Check for 1GB_PG:\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_1GB_PG);
-
-       printf("Check for INVTSC:\t");
-       CHECK_FOR_FLAG(RTE_CPUFLAG_INVTSC);
-
-
-#endif
+       for (i = 0; i < RTE_CPUFLAG_NUMFLAGS; i++) {
+               printf("Check for %s:\t\t", cpu_feature_table[i].name);
+               CHECK_FOR_FLAG(i);
+       }

Thanks
-Zhigang