[dpdk-dev] [PATCH v1] mk: allow use of toolchain cflags

[dpdk-dev] [PATCH v1] mk: allow use of toolchain cflags

[David Hunt]

From: Vipin Varghese <vipin.varghese@intel.com>

with 'export TOOLCHAIN_CFLAGS' the values are
not inherited throuhout make system. The change
appends the user defined cflags.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
Signed-off-by: David Hunt <david.hunt@intel.com>
---
 mk/toolchain/clang/rte.vars.mk | 6 +++---
 mk/toolchain/gcc/rte.vars.mk   | 6 +++---
 mk/toolchain/icc/rte.vars.mk   | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/mk/toolchain/clang/rte.vars.mk b/mk/toolchain/clang/rte.vars.mk
index dde922d..1b017db 100644
--- a/mk/toolchain/clang/rte.vars.mk
+++ b/mk/toolchain/clang/rte.vars.mk
@@ -59,9 +59,9 @@ HOSTCC    = clang
 endif
 HOSTAS    = as
 
-TOOLCHAIN_ASFLAGS =
-TOOLCHAIN_CFLAGS =
-TOOLCHAIN_LDFLAGS =
+TOOLCHAIN_ASFLAGS +=
+TOOLCHAIN_CFLAGS +=
+TOOLCHAIN_LDFLAGS +=
 
 WERROR_FLAGS := -W -Wall -Wstrict-prototypes -Wmissing-prototypes
 WERROR_FLAGS += -Wmissing-declarations -Wold-style-definition -Wpointer-arith
diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
index 3b907e2..e52da4d 100644
--- a/mk/toolchain/gcc/rte.vars.mk
+++ b/mk/toolchain/gcc/rte.vars.mk
@@ -59,9 +59,9 @@ HOSTCC    = gcc
 endif
 HOSTAS    = as
 
-TOOLCHAIN_ASFLAGS =
-TOOLCHAIN_CFLAGS =
-TOOLCHAIN_LDFLAGS =
+TOOLCHAIN_ASFLAGS +=
+TOOLCHAIN_CFLAGS +=
+TOOLCHAIN_LDFLAGS +=
 
 ifeq ($(CONFIG_RTE_LIBRTE_GCOV),y)
 TOOLCHAIN_CFLAGS += --coverage
diff --git a/mk/toolchain/icc/rte.vars.mk b/mk/toolchain/icc/rte.vars.mk
index 33a8ba7..70984b7 100644
--- a/mk/toolchain/icc/rte.vars.mk
+++ b/mk/toolchain/icc/rte.vars.mk
@@ -59,9 +59,9 @@ HOSTCC    = gcc
 endif
 HOSTAS    = as
 
-TOOLCHAIN_CFLAGS =
-TOOLCHAIN_LDFLAGS =
-TOOLCHAIN_ASFLAGS =
+TOOLCHAIN_CFLAGS +=
+TOOLCHAIN_LDFLAGS +=
+TOOLCHAIN_ASFLAGS +=
 
 # Turn off some ICC warnings -
 #   Remark #271   : trailing comma is nonstandard
-- 
2.7.4

Re: [dpdk-dev] [PATCH v1] mk: allow use of toolchain cflags

[Thomas Monjalon]

03/11/2017 10:40, David Hunt:
> From: Vipin Varghese <vipin.varghese@intel.com>
> 
> with 'export TOOLCHAIN_CFLAGS' the values are
> not inherited throuhout make system. The change
> appends the user defined cflags.

Why do you want to use export from the shell environment?
The standard way of overriding a variable with make,
is to specify a value on the make command line.

[...]
> -TOOLCHAIN_ASFLAGS =
> -TOOLCHAIN_CFLAGS =
> -TOOLCHAIN_LDFLAGS =
> +TOOLCHAIN_ASFLAGS +=
> +TOOLCHAIN_CFLAGS +=
> +TOOLCHAIN_LDFLAGS +=

There is nothing to append here.
It is here only for documentation purpose.
If you really want to take environment value into account,
you can just comment this assignment or use ?= operator.

Note that you can also use EXTRA_CFLAGS.
I would be in favor of removing these variables.
We have too many variables.

Re: [dpdk-dev] [PATCH v1] mk: allow use of toolchain cflags

[Varghese, Vipin]

Hi Thomas,

Please find my answers below 

Why do you want to use export from the shell environment?
Answer> One scenario that generic scripts or makefiles are used to build DPDK and Customer components. While building for "make debug" mode; we require DPDK libraries to be built with compiler specific additives. Hence there are requirements of having them exported (especially in DPDK shared library mode) is in practice.

The standard way of overriding a variable with make, is to specify a value on the make command line.
Answer> Please find my observations while using non modified DPDK versions

CMD: 
1) cd $RTE_SDK/$RTE_TARGET
2) make clean; make TOOLCHAIN_CFLAGS="-ggdb -fno-omit-frame-pointer"

Checking for user flags:
# grep -nr "fno-omit-frame-pointer" lib/ | wc -l
	66
<these were not expected>
# grep -nr "fno-omit-frame-pointer" app/
	Binary file app/dpdk-procinfo matches
	Binary file app/testpmd matches
	Binary file app/dpdk-test-eventdev matches
	Binary file app/dpdk-test-crypto-perf matches
	Binary file app/dpdk-pdump matches
# grep -nr "fno-omit-frame-pointer" kmod/
<confirming application are shared library, hence code segment should not have been built with these flags>
# ldd x86_64-native-linuxapp-gcc/app/* | grep rte_eal
        	librte_eal.so.5.1 => /home/saesrv02/Downloads/dpdksrc/dpdk-17.08/x86_64-native-linuxapp-gcc/lib/librte_eal.so.5.1 (0x00007f9a7badc000)
        	librte_eal.so.5.1 => /home/saesrv02/Downloads/dpdksrc/dpdk-17.08/x86_64-native-linuxapp-gcc/lib/librte_eal.so.5.1 (0x00007fb2fb318000)
        	librte_eal.so.5.1 => /home/saesrv02/Downloads/dpdksrc/dpdk-17.08/x86_64-native-linuxapp-gcc/lib/librte_eal.so.5.1 (0x00007f865530f000)
        	librte_eal.so.5.1 => /home/saesrv02/Downloads/dpdksrc/dpdk-17.08/x86_64-native-linuxapp-gcc/lib/librte_eal.so.5.1 (0x00007f83c7c4c000)
        	librte_eal.so.5.1 => /home/saesrv02/Downloads/dpdksrc/dpdk-17.08/x86_64-native-linuxapp-gcc/lib/librte_eal.so.5.1 (0x00007f1003c10000)

If you really want to take environment value into account, you can just comment this assignment or use ?= operator.
Answer> I also tried adding variable into ".config" - "CONFIG_RTE_TOOLCHAIN_GCC_DEBUG=y", then manually editing "mk/toolchain/gcc/rte.vars.mk" as "ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC_DEBUG),y) TOOLCHAIN_CFLAGS ?= <my desired flags>". But this requires to edit or sed the file for each new combination. Hence dropped this approach.

Note that you can also use EXTRA_CFLAGS.
Answer> As per the mailing list http://dpdk.org/ml/archives/dev/2015-August/022592.html, passing EXTRA_CFLAGS will solve the issue. But this affect modules, libraries and application
grep -nr EXTRA_CFLAGS * --include=*.mk
internal/rte.compile-pre.mk:78: $(CFLAGS_$(@)) $(HOST_EXTRA_CPPFLAGS) $(HOST_EXTRA_CFLAGS) -o $@ -c $<
internal/rte.compile-pre.mk:83: $(CFLAGS_$(@)) $(EXTRA_CPPFLAGS) $(EXTRA_CFLAGS) -o $@ -c $<
rte.app.mk:245:O_TO_EXE = $(CC) -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $(OBJS-y) $(call linkerprefix, \
rte.bsdmodule.mk:34:override EXTRA_CFLAGS = $(MODULE_CFLAGS) $(EXTRA_KERNEL_CFLAGS)
rte.cpuflags.mk:36:AUTO_CPUFLAGS := $(shell $(CC) $(MACHINE_CFLAGS) $(WERROR_FLAGS) $(EXTRA_CFLAGS) -dM -E - < /dev/null)
rte.lib.mk:80:LD := $(CC) $(CPU_CFLAGS) $(EXTRA_CFLAGS)
rte.module.mk:34:override EXTRA_CFLAGS = $(MODULE_CFLAGS) $(EXTRA_KERNEL_CFLAGS)
toolchain/gcc/rte.vars.mk:69:ifeq (,$(findstring -O0,$(EXTRA_CFLAGS)))
toolchain/gcc/rte.vars.mk:70: $(warning "EXTRA_CFLAGS doesn't contains -O0, coverage will be inaccurate with optimizations enabled")

I would be in favor of removing these variables.
We have too many variables.
Answer> Ok, at least we should have right variables to allow build components with right settings.

Thanks
Vipin Varghese

-----Original Message-----
From: Thomas Monjalon [mailto:thomas@monjalon.net] 
Sent: Tuesday, November 7, 2017 2:38 AM
To: Hunt, David <david.hunt@intel.com>; Varghese, Vipin <vipin.varghese@intel.com>
Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
Subject: Re: [dpdk-dev] [PATCH v1] mk: allow use of toolchain cflags

03/11/2017 10:40, David Hunt:
> From: Vipin Varghese <vipin.varghese@intel.com>
> 
> with 'export TOOLCHAIN_CFLAGS' the values are not inherited throuhout 
> make system. The change appends the user defined cflags.

Why do you want to use export from the shell environment?
The standard way of overriding a variable with make, is to specify a value on the make command line.

[...]
> -TOOLCHAIN_ASFLAGS =
> -TOOLCHAIN_CFLAGS =
> -TOOLCHAIN_LDFLAGS =
> +TOOLCHAIN_ASFLAGS +=
> +TOOLCHAIN_CFLAGS +=
> +TOOLCHAIN_LDFLAGS +=

There is nothing to append here.
It is here only for documentation purpose.
If you really want to take environment value into account, you can just comment this assignment or use ?= operator.

Note that you can also use EXTRA_CFLAGS.
I would be in favor of removing these variables.
We have too many variables.