[dpdk-dev] acl: fix target arch detection

Message ID 1443247147-5763-1-git-send-email-zoltan.kiss@linaro.org (mailing list archive)
State Rejected, archived
Headers

Commit Message

Zoltan Kiss Sept. 26, 2015, 5:59 a.m. UTC
  This test selects AVX2 code even if the target architecture doesn't support it.

Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
---
  

Comments

Ananyev, Konstantin Sept. 29, 2015, 9:42 a.m. UTC | #1
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?
  

Patch

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)