[dpdk-dev] [PATCH] acl: fix target arch detection

[dpdk-dev] [PATCH] acl: fix target arch detection

[Zoltan Kiss]

This test selects AVX2 code even if the target architecture doesn't support it.

Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
---
diff --git a/lib/librte_acl/Makefile b/lib/librte_acl/Makefile
index 46acc2b..17a9f96 100644
--- a/lib/librte_acl/Makefile
+++ b/lib/librte_acl/Makefile
@@ -57,7 +57,7 @@ CFLAGS_acl_run_sse.o += -msse4.1
 # then add support for AVX2 classify method.
 #
 
-CC_AVX2_SUPPORT=$(shell $(CC) -march=core-avx2 -dM -E - </dev/null 2>&1 | \
+CC_AVX2_SUPPORT=$(shell $(CC) $(MACHINE_FLAGS) -dM -E - </dev/null 2>&1 | \
 grep -q AVX2 && echo 1)
 
 ifeq ($(CC_AVX2_SUPPORT), 1)

Re: [dpdk-dev] [PATCH] acl: fix target arch detection

[Ananyev, Konstantin]

Hi Zoltan,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zoltan Kiss
> Sent: Saturday, September 26, 2015 6:59 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] acl: fix target arch detection
> 
> This test selects AVX2 code even if the target architecture doesn't support it.
> 
> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
> ---
> diff --git a/lib/librte_acl/Makefile b/lib/librte_acl/Makefile
> index 46acc2b..17a9f96 100644
> --- a/lib/librte_acl/Makefile
> +++ b/lib/librte_acl/Makefile
> @@ -57,7 +57,7 @@ CFLAGS_acl_run_sse.o += -msse4.1
>  # then add support for AVX2 classify method.
>  #
> 
> -CC_AVX2_SUPPORT=$(shell $(CC) -march=core-avx2 -dM -E - </dev/null 2>&1 | \
> +CC_AVX2_SUPPORT=$(shell $(CC) $(MACHINE_FLAGS) -dM -E - </dev/null 2>&1 | \
>  grep -q AVX2 && echo 1)
> 
>  ifeq ($(CC_AVX2_SUPPORT), 1)

The purpose of that code is to check does compiler supports AVX2 or not.
If it does, even if selected target doesn't support AVX2, we still compile in AVX2  version of acl_classify (same for SSE4).
Then  at startup, rte_acl_init() selects the highest supported on that box implementation (scalar/sse/avx2).
So NACK.
Konstantin

BTW, why MACHINE_FLAGS?
Shouldn't it be MACHINE_CFLAGS, instead?