diff mbox

Re: [PATCH] Makefiles: Allow CFLAGS to overide the build

Message ID 20140218112207.GD3517@8470w
State Rejected, archived
Headers show

Commit Message

Anders Roxell Feb. 18, 2014, 11:22 a.m. UTC
On 2014-02-17 23:55, Petri Savolainen wrote:
> 
> 
> On Tuesday, 18 February 2014 00:16:29 UTC+2, Mike Holmes wrote:
> >
> > Signed-off-by: Mike Holmes <mike....@linaro.org <javascript:>> 
> > --- 
> >
> > CFLAGS is usually expected to take effect on a build if applied from the 
> > command line when calling make, or if it is set in the environment. 
> >
> > Examples are when building with CLANG, -Wno-error must be applied to 
> > allow CLANG warnings which are a superset of gcc to cause the build to 
> > fail. e.g. make CC=clang CFLAGS=-Wno-error 
> >
> > When building with optimizations off and with -g for debugging, it must 
> > be possible to alter the defaults, in ODPs case these defaults are in 
> > Makefile.inc 
> >
> > e.g. make CFLAGS=-O3 
> >
> >  .checkpatch.conf                |  1 + 
> >  Makefile.inc                    | 16 ++++++++-------- 
> >  platform/linux-generic/Makefile | 10 +++++----- 
> >  test/api_test/Makefile          |  6 +++--- 
> >  test/example/Makefile           |  2 +- 
> >  test/packet/Makefile            |  4 ++-- 
> >  test/packet_netmap/Makefile     |  6 +++--- 
> >  7 files changed, 23 insertions(+), 22 deletions(-) 
> >
> > diff --git a/.checkpatch.conf b/.checkpatch.conf 
> > index e1a25c8..9076410 100644 
> > --- a/.checkpatch.conf 
> > +++ b/.checkpatch.conf 
> > @@ -1,3 +1,4 @@ 
> >  --no-tree 
> >  --strict 
> >  --ignore=NEW_TYPEDEFS 
> > +--ignore=DEPRECATED_VARIABLE 
> > diff --git a/Makefile.inc b/Makefile.inc 
> > index 7dea028..75a4829 100644 
> > --- a/Makefile.inc 
> > +++ b/Makefile.inc 
> > @@ -4,17 +4,17 @@ 
> >  # SPDX-License-Identifier:        BSD-3-Clause 
> >   
> >  PLATFORM ?= linux-generic 
> > -CFLAGS  += -DODP_DEBUG=1 
> > -#CFLAGS  += -O3 
> > -CFLAGS  += -O0 -g 
> > +EXTRA_CFLAGS  += -DODP_DEBUG=1 
> > +#EXTRA_CFLAGS  += -O3 
> > +EXTRA_CFLAGS  += -O0 -g 
> >
> 
> As I said earlier, -O3 must be used by default. It must be used in 
> regression testing. Otherwise we have code soon that does not compile with 
> -O3, or compiles but has bugs with optimization.
> 
> -Petri
>  
> 
> >   
> >  OBJ_DIR  = ./obj 
> >  DESTDIR ?= $(ODP_ROOT)/build 
> >   
> > -CFLAGS += -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes 
> > -CFLAGS += -Wmissing-declarations -Wold-style-definition -Wpointer-arith 
> > -CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual -Wformat-nonliteral 
> > -CFLAGS += -Wformat-security -Wundef -Wwrite-strings 
> > +EXTRA_CFLAGS += -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes 
> > +EXTRA_CFLAGS += -Wmissing-declarations -Wold-style-definition 
> > -Wpointer-arith 
> > +EXTRA_CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual 
> > -Wformat-nonliteral 
> > +EXTRA_CFLAGS += -Wformat-security -Wundef -Wwrite-strings 
> >   
> >  CC     ?= gcc 
> >  LD     ?= gcc 
> > @@ -26,5 +26,5 @@ RMDIR  := rm -rf 
> >  RM     := rm -f 
> >  COPY   := cp -r 
> >   
> > -CFLAGS  += -pthread 
> > +EXTRA_EXTRA_CFLAGS  += -pthread 
> >  LDFLAGS += -pthread 
> > diff --git a/platform/linux-generic/Makefile 
> > b/platform/linux-generic/Makefile 
> > index f665683..9828ee4 100644 
> > --- a/platform/linux-generic/Makefile 
> > +++ b/platform/linux-generic/Makefile 
> > @@ -32,12 +32,12 @@ ODP_ROOT = ../.. 
> >  LIB_DIR  = ./lib 
> >  DOC_DIR  = ./doc 
> >   
> > -CFLAGS  += -I$(ODP_ROOT)/include 
> > -CFLAGS  += -I./include 
> > -CFLAGS  += -I./include/api 
> > +EXTRA_CFLAGS  += -I$(ODP_ROOT)/include 
> > +EXTRA_CFLAGS  += -I./include 
> > +EXTRA_CFLAGS  += -I./include/api 
> >   
> >  ifeq ($(ODP_HAVE_NETMAP),yes) 
> > -CFLAGS  += -DODP_HAVE_NETMAP 
> > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP 
> >  endif 
> >   
> >  include $(ODP_ROOT)/Makefile.inc 
> > @@ -92,7 +92,7 @@ $(DOC_DIR): 
> >  # 
> >  $(OBJ_DIR)/%.o: ./source/%.c 
> >          $(ECHO) Compiling $< 
> > -        $(CC) -c -MD $(CFLAGS) -o $@ $< 
> > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $< 
> >   
> >  # 
> >  # Lib rule 
> > diff --git a/test/api_test/Makefile b/test/api_test/Makefile 
> > index 15c18f4..741fbe4 100644 
> > --- a/test/api_test/Makefile 
> > +++ b/test/api_test/Makefile 
> > @@ -12,12 +12,12 @@ ODP_ATOMIC    = odp_atomic 
> >  ODP_SHM       = odp_shm 
> >  ODP_RING      = odp_ring 
> >   
> > -CFLAGS  += -I$(ODP_ROOT)/platform/linux-generic/include 
> > +EXTRA_CFLAGS  += -I$(ODP_ROOT)/platform/linux-generic/include 
> >   
> >  include ../Makefile.inc 
> >  include $(ODP_ROOT)/Makefile.inc 
> >   
> > -CFLAGS      += -I$(ODP_TEST_ROOT)/api_test 
> > +EXTRA_CFLAGS      += -I$(ODP_TEST_ROOT)/api_test 
> >   
> >  ATOMIC_OBJS  = 
> >  ATOMIC_OBJS += $(OBJ_DIR)/odp_common.o 
> > @@ -52,7 +52,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a 
> >  # 
> >  $(OBJ_DIR)/%.o: %.c 
> >          $(ECHO) Compiling $< 
> > -        $(CC) -c -MD $(CFLAGS) -o $@ $< 
> > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $< 
> >   
> >  # 
> >  # Link rule 
> > diff --git a/test/example/Makefile b/test/example/Makefile 
> > index d43e780..8064977 100644 
> > --- a/test/example/Makefile 
> > +++ b/test/example/Makefile 
> > @@ -30,7 +30,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a 
> >  # 
> >  $(OBJ_DIR)/%.o: %.c 
> >          $(ECHO) Compiling $< 
> > -        $(CC) -c -MD $(CFLAGS) -o $@ $< 
> > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $< 
> >   
> >  # 
> >  # Link rule 
> > diff --git a/test/packet/Makefile b/test/packet/Makefile 
> > index f1cb7d9..c66b37c 100644 
> > --- a/test/packet/Makefile 
> > +++ b/test/packet/Makefile 
> > @@ -9,7 +9,7 @@ ODP_APP  = odp_packet 
> >  include ../Makefile.inc 
> >  include $(ODP_ROOT)/Makefile.inc 
> >   
> > -CFLAGS  += -I$(ODP_TEST_ROOT)/packet 
> > +EXTRA_CFLAGS  += -I$(ODP_TEST_ROOT)/packet 
> >   
> >  OBJS     = 
> >  OBJS    += $(OBJ_DIR)/odp_example_pktio.o 
> > @@ -32,7 +32,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a 
> >  # 
> >  $(OBJ_DIR)/%.o: %.c 
> >          $(ECHO) Compiling $< 
> > -        $(CC) -c -MD $(CFLAGS) -o $@ $< 
> > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $< 
> >   
> >  # 
> >  # Link rule 
> > diff --git a/test/packet_netmap/Makefile b/test/packet_netmap/Makefile 
> > index 5febb33..d92d113 100644 
> > --- a/test/packet_netmap/Makefile 
> > +++ b/test/packet_netmap/Makefile 
> > @@ -6,8 +6,8 @@ 
> >  ODP_ROOT = ../.. 
> >  ODP_APP  = odp_packet 
> >   
> > -CFLAGS  += -DODP_HAVE_NETMAP 
> > -CFLAGS  += -O0 -g 
> > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP 
> > +EXTRA_CFLAGS  += -O0 -g 
> >   
> >  include ../Makefile.inc 
> >  include $(ODP_ROOT)/Makefile.inc 
> > @@ -33,7 +33,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a 
> >  # 
> >  $(OBJ_DIR)/%.o: %.c 
> >          $(ECHO) Compiling $< 
> > -        $(CC) -c -MD $(CFLAGS) -o $@ $< 
> > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $< 
> >   
> >  # 
> >  # Link rule 
> > -- 
> > 1.8.3.2 
> >
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups "LNG ODP Sub-team - lng-odp@linaro.org" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to lng-odp+unsubscribe@linaro.org.
> To post to this group, send email to lng-odp@linaro.org.
> Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/.
> To view this discussion on the web visit https://groups.google.com/a/linaro.org/d/msgid/lng-odp/2b98325d-9086-4a5c-ada0-f5c7dfe81e17%40linaro.org.
> For more options, visit https://groups.google.com/a/linaro.org/groups/opt_out.

Petri Mike,

Would this solve the problems?

For clang you need to specify: make CC=clang EXTRA_CFLAGS=-Wno-error
would that work?

Drawback with this solution is that we can't use -O3 + ODP_DBG macro, do
we want that? if yes, then we just need to change:
ifeq ($(ODP_DEBUG), 1) to ifeq ($(a_good_name), 1)
ideas of a good name?

Comments

Mike Holmes Feb. 18, 2014, 2:43 p.m. UTC | #1
"As I said earlier, -O3 must be used by default. It must be used in
regression testing"
Nothing I propose alters the defaults, they just allow things to be changed
as needed, for regression we need to also compile with debug on and this
allow that permutation.


On 18 February 2014 06:22, Anders Roxell <anders.roxell@linaro.org> wrote:

> On 2014-02-17 23:55, Petri Savolainen wrote:
> >
> >
> > On Tuesday, 18 February 2014 00:16:29 UTC+2, Mike Holmes wrote:
> > >
> > > Signed-off-by: Mike Holmes <mike....@linaro.org <javascript:>>
> > > ---
> > >
> > > CFLAGS is usually expected to take effect on a build if applied from
> the
> > > command line when calling make, or if it is set in the environment.
> > >
> > > Examples are when building with CLANG, -Wno-error must be applied to
> > > allow CLANG warnings which are a superset of gcc to cause the build to
> > > fail. e.g. make CC=clang CFLAGS=-Wno-error
> > >
> > > When building with optimizations off and with -g for debugging, it must
> > > be possible to alter the defaults, in ODPs case these defaults are in
> > > Makefile.inc
> > >
> > > e.g. make CFLAGS=-O3
> > >
> > >  .checkpatch.conf                |  1 +
> > >  Makefile.inc                    | 16 ++++++++--------
> > >  platform/linux-generic/Makefile | 10 +++++-----
> > >  test/api_test/Makefile          |  6 +++---
> > >  test/example/Makefile           |  2 +-
> > >  test/packet/Makefile            |  4 ++--
> > >  test/packet_netmap/Makefile     |  6 +++---
> > >  7 files changed, 23 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/.checkpatch.conf b/.checkpatch.conf
> > > index e1a25c8..9076410 100644
> > > --- a/.checkpatch.conf
> > > +++ b/.checkpatch.conf
> > > @@ -1,3 +1,4 @@
> > >  --no-tree
> > >  --strict
> > >  --ignore=NEW_TYPEDEFS
> > > +--ignore=DEPRECATED_VARIABLE
> > > diff --git a/Makefile.inc b/Makefile.inc
> > > index 7dea028..75a4829 100644
> > > --- a/Makefile.inc
> > > +++ b/Makefile.inc
> > > @@ -4,17 +4,17 @@
> > >  # SPDX-License-Identifier:        BSD-3-Clause
> > >
> > >  PLATFORM ?= linux-generic
> > > -CFLAGS  += -DODP_DEBUG=1
> > > -#CFLAGS  += -O3
> > > -CFLAGS  += -O0 -g
> > > +EXTRA_CFLAGS  += -DODP_DEBUG=1
> > > +#EXTRA_CFLAGS  += -O3
> > > +EXTRA_CFLAGS  += -O0 -g
> > >
> >
> > As I said earlier, -O3 must be used by default. It must be used in
> > regression testing. Otherwise we have code soon that does not compile
> with
> > -O3, or compiles but has bugs with optimization.
> >
> > -Petri
> >
> >
> > >
> > >  OBJ_DIR  = ./obj
> > >  DESTDIR ?= $(ODP_ROOT)/build
> > >
> > > -CFLAGS += -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes
> > > -CFLAGS += -Wmissing-declarations -Wold-style-definition
> -Wpointer-arith
> > > -CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
> -Wformat-nonliteral
> > > -CFLAGS += -Wformat-security -Wundef -Wwrite-strings
> > > +EXTRA_CFLAGS += -W -Wall -Werror -Wstrict-prototypes
> -Wmissing-prototypes
> > > +EXTRA_CFLAGS += -Wmissing-declarations -Wold-style-definition
> > > -Wpointer-arith
> > > +EXTRA_CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
> > > -Wformat-nonliteral
> > > +EXTRA_CFLAGS += -Wformat-security -Wundef -Wwrite-strings
> > >
> > >  CC     ?= gcc
> > >  LD     ?= gcc
> > > @@ -26,5 +26,5 @@ RMDIR  := rm -rf
> > >  RM     := rm -f
> > >  COPY   := cp -r
> > >
> > > -CFLAGS  += -pthread
> > > +EXTRA_EXTRA_CFLAGS  += -pthread
> > >  LDFLAGS += -pthread
> > > diff --git a/platform/linux-generic/Makefile
> > > b/platform/linux-generic/Makefile
> > > index f665683..9828ee4 100644
> > > --- a/platform/linux-generic/Makefile
> > > +++ b/platform/linux-generic/Makefile
> > > @@ -32,12 +32,12 @@ ODP_ROOT = ../..
> > >  LIB_DIR  = ./lib
> > >  DOC_DIR  = ./doc
> > >
> > > -CFLAGS  += -I$(ODP_ROOT)/include
> > > -CFLAGS  += -I./include
> > > -CFLAGS  += -I./include/api
> > > +EXTRA_CFLAGS  += -I$(ODP_ROOT)/include
> > > +EXTRA_CFLAGS  += -I./include
> > > +EXTRA_CFLAGS  += -I./include/api
> > >
> > >  ifeq ($(ODP_HAVE_NETMAP),yes)
> > > -CFLAGS  += -DODP_HAVE_NETMAP
> > > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP
> > >  endif
> > >
> > >  include $(ODP_ROOT)/Makefile.inc
> > > @@ -92,7 +92,7 @@ $(DOC_DIR):
> > >  #
> > >  $(OBJ_DIR)/%.o: ./source/%.c
> > >          $(ECHO) Compiling $<
> > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
> > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
> > >
> > >  #
> > >  # Lib rule
> > > diff --git a/test/api_test/Makefile b/test/api_test/Makefile
> > > index 15c18f4..741fbe4 100644
> > > --- a/test/api_test/Makefile
> > > +++ b/test/api_test/Makefile
> > > @@ -12,12 +12,12 @@ ODP_ATOMIC    = odp_atomic
> > >  ODP_SHM       = odp_shm
> > >  ODP_RING      = odp_ring
> > >
> > > -CFLAGS  += -I$(ODP_ROOT)/platform/linux-generic/include
> > > +EXTRA_CFLAGS  += -I$(ODP_ROOT)/platform/linux-generic/include
> > >
> > >  include ../Makefile.inc
> > >  include $(ODP_ROOT)/Makefile.inc
> > >
> > > -CFLAGS      += -I$(ODP_TEST_ROOT)/api_test
> > > +EXTRA_CFLAGS      += -I$(ODP_TEST_ROOT)/api_test
> > >
> > >  ATOMIC_OBJS  =
> > >  ATOMIC_OBJS += $(OBJ_DIR)/odp_common.o
> > > @@ -52,7 +52,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
> > >  #
> > >  $(OBJ_DIR)/%.o: %.c
> > >          $(ECHO) Compiling $<
> > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
> > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
> > >
> > >  #
> > >  # Link rule
> > > diff --git a/test/example/Makefile b/test/example/Makefile
> > > index d43e780..8064977 100644
> > > --- a/test/example/Makefile
> > > +++ b/test/example/Makefile
> > > @@ -30,7 +30,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
> > >  #
> > >  $(OBJ_DIR)/%.o: %.c
> > >          $(ECHO) Compiling $<
> > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
> > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
> > >
> > >  #
> > >  # Link rule
> > > diff --git a/test/packet/Makefile b/test/packet/Makefile
> > > index f1cb7d9..c66b37c 100644
> > > --- a/test/packet/Makefile
> > > +++ b/test/packet/Makefile
> > > @@ -9,7 +9,7 @@ ODP_APP  = odp_packet
> > >  include ../Makefile.inc
> > >  include $(ODP_ROOT)/Makefile.inc
> > >
> > > -CFLAGS  += -I$(ODP_TEST_ROOT)/packet
> > > +EXTRA_CFLAGS  += -I$(ODP_TEST_ROOT)/packet
> > >
> > >  OBJS     =
> > >  OBJS    += $(OBJ_DIR)/odp_example_pktio.o
> > > @@ -32,7 +32,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
> > >  #
> > >  $(OBJ_DIR)/%.o: %.c
> > >          $(ECHO) Compiling $<
> > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
> > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
> > >
> > >  #
> > >  # Link rule
> > > diff --git a/test/packet_netmap/Makefile b/test/packet_netmap/Makefile
> > > index 5febb33..d92d113 100644
> > > --- a/test/packet_netmap/Makefile
> > > +++ b/test/packet_netmap/Makefile
> > > @@ -6,8 +6,8 @@
> > >  ODP_ROOT = ../..
> > >  ODP_APP  = odp_packet
> > >
> > > -CFLAGS  += -DODP_HAVE_NETMAP
> > > -CFLAGS  += -O0 -g
> > > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP
> > > +EXTRA_CFLAGS  += -O0 -g
> > >
> > >  include ../Makefile.inc
> > >  include $(ODP_ROOT)/Makefile.inc
> > > @@ -33,7 +33,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
> > >  #
> > >  $(OBJ_DIR)/%.o: %.c
> > >          $(ECHO) Compiling $<
> > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
> > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
> > >
> > >  #
> > >  # Link rule
> > > --
> > > 1.8.3.2
> > >
> > >
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups "LNG ODP Sub-team - lng-odp@linaro.org" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an email to lng-odp+unsubscribe@linaro.org.
> > To post to this group, send email to lng-odp@linaro.org.
> > Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/
> .
> > To view this discussion on the web visit
> https://groups.google.com/a/linaro.org/d/msgid/lng-odp/2b98325d-9086-4a5c-ada0-f5c7dfe81e17%40linaro.org
> .
> > For more options, visit
> https://groups.google.com/a/linaro.org/groups/opt_out.
>
> Petri Mike,
>
> Would this solve the problems?
>
> For clang you need to specify: make CC=clang EXTRA_CFLAGS=-Wno-error
> would that work?
>
> Drawback with this solution is that we can't use -O3 + ODP_DBG macro, do
> we want that? if yes, then we just need to change:
> ifeq ($(ODP_DEBUG), 1) to ifeq ($(a_good_name), 1)
> ideas of a good name?
>
> diff --git a/Makefile.inc b/Makefile.inc
> index 523385d..b35e030 100644
> --- a/Makefile.inc
> +++ b/Makefile.inc
> @@ -4,17 +4,24 @@
>  # SPDX-License-Identifier:     BSD-3-Clause
>
>  PLATFORM ?= linux-generic
> -CFLAGS  += -DODP_DEBUG=1
> -CFLAGS  += -O3
> -#CFLAGS  += -O0 -g
> +ODP_DEBUG ?= 0
>
>  OBJ_DIR  = ./obj
>  DESTDIR ?= $(ODP_ROOT)/build
> +EXTRA_CFLAGS ?=
>
>  CFLAGS += -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes
>  CFLAGS += -Wmissing-declarations -Wold-style-definition -Wpointer-arith
>  CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual -Wformat-nonliteral
>  CFLAGS += -Wformat-security -Wundef -Wwrite-strings
> +CFLAGS += $(EXTRA_CFLAGS)
> +CFLAGS += -DODP_DEBUG=$(ODP_DEBUG)
> +
> +ifeq ($(ODP_DEBUG), 1)
> +  CFLAGS  += -O0 -g
> +else
> +  CFLAGS  += -O3
> +endif
>
>  CC     ?= gcc
>  LD     ?= gcc
>
>
> --
> Anders Roxell
> anders.roxell@linaro.org
> M: +46 709 71 42 85 | IRC: roxell
>
> --
> You received this message because you are subscribed to the Google Groups
> "LNG ODP Sub-team - lng-odp@linaro.org" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to lng-odp+unsubscribe@linaro.org.
> To post to this group, send email to lng-odp@linaro.org.
> Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/.
> To view this discussion on the web visit
> https://groups.google.com/a/linaro.org/d/msgid/lng-odp/20140218112207.GD3517%408470w
> .
> For more options, visit
> https://groups.google.com/a/linaro.org/groups/opt_out.
>
Mike Holmes Feb. 18, 2014, 6:37 p.m. UTC | #2
Anders,

Look up gnu Make recommendations, they suggest that a variable like
EXTRA_FLAGS be the internal variable with the defaults, and that CFLAGS be
the one a user would alter if they want to change the optimization level
etc from the default.

I am not a fan of adding ODP_DEBUG, I think it is just as easy, if
I don't want the default to say
"make CFLAGS=-O0 -g "

That way we don't clutter our make with special cases, if we open the dor
to ODP_DEBUG,  we will start adding ODP_TRACING to add the gcov and gcov
flags and it is a slippery slope of special case from there.


On 18 February 2014 09:43, Mike Holmes <mike.holmes@linaro.org> wrote:

> "As I said earlier, -O3 must be used by default. It must be used in
> regression testing"
> Nothing I propose alters the defaults, they just allow things to be
> changed as needed, for regression we need to also compile with debug on and
> this allow that permutation.
>
>
> On 18 February 2014 06:22, Anders Roxell <anders.roxell@linaro.org> wrote:
>
>> On 2014-02-17 23:55, Petri Savolainen wrote:
>> >
>> >
>> > On Tuesday, 18 February 2014 00:16:29 UTC+2, Mike Holmes wrote:
>> > >
>> > > Signed-off-by: Mike Holmes <mike....@linaro.org <javascript:>>
>> > > ---
>> > >
>> > > CFLAGS is usually expected to take effect on a build if applied from
>> the
>> > > command line when calling make, or if it is set in the environment.
>> > >
>> > > Examples are when building with CLANG, -Wno-error must be applied to
>> > > allow CLANG warnings which are a superset of gcc to cause the build to
>> > > fail. e.g. make CC=clang CFLAGS=-Wno-error
>> > >
>> > > When building with optimizations off and with -g for debugging, it
>> must
>> > > be possible to alter the defaults, in ODPs case these defaults are in
>> > > Makefile.inc
>> > >
>> > > e.g. make CFLAGS=-O3
>> > >
>> > >  .checkpatch.conf                |  1 +
>> > >  Makefile.inc                    | 16 ++++++++--------
>> > >  platform/linux-generic/Makefile | 10 +++++-----
>> > >  test/api_test/Makefile          |  6 +++---
>> > >  test/example/Makefile           |  2 +-
>> > >  test/packet/Makefile            |  4 ++--
>> > >  test/packet_netmap/Makefile     |  6 +++---
>> > >  7 files changed, 23 insertions(+), 22 deletions(-)
>> > >
>> > > diff --git a/.checkpatch.conf b/.checkpatch.conf
>> > > index e1a25c8..9076410 100644
>> > > --- a/.checkpatch.conf
>> > > +++ b/.checkpatch.conf
>> > > @@ -1,3 +1,4 @@
>> > >  --no-tree
>> > >  --strict
>> > >  --ignore=NEW_TYPEDEFS
>> > > +--ignore=DEPRECATED_VARIABLE
>> > > diff --git a/Makefile.inc b/Makefile.inc
>> > > index 7dea028..75a4829 100644
>> > > --- a/Makefile.inc
>> > > +++ b/Makefile.inc
>> > > @@ -4,17 +4,17 @@
>> > >  # SPDX-License-Identifier:        BSD-3-Clause
>> > >
>> > >  PLATFORM ?= linux-generic
>> > > -CFLAGS  += -DODP_DEBUG=1
>> > > -#CFLAGS  += -O3
>> > > -CFLAGS  += -O0 -g
>> > > +EXTRA_CFLAGS  += -DODP_DEBUG=1
>> > > +#EXTRA_CFLAGS  += -O3
>> > > +EXTRA_CFLAGS  += -O0 -g
>> > >
>> >
>> > As I said earlier, -O3 must be used by default. It must be used in
>> > regression testing. Otherwise we have code soon that does not compile
>> with
>> > -O3, or compiles but has bugs with optimization.
>> >
>> > -Petri
>> >
>> >
>> > >
>> > >  OBJ_DIR  = ./obj
>> > >  DESTDIR ?= $(ODP_ROOT)/build
>> > >
>> > > -CFLAGS += -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes
>> > > -CFLAGS += -Wmissing-declarations -Wold-style-definition
>> -Wpointer-arith
>> > > -CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
>> -Wformat-nonliteral
>> > > -CFLAGS += -Wformat-security -Wundef -Wwrite-strings
>> > > +EXTRA_CFLAGS += -W -Wall -Werror -Wstrict-prototypes
>> -Wmissing-prototypes
>> > > +EXTRA_CFLAGS += -Wmissing-declarations -Wold-style-definition
>> > > -Wpointer-arith
>> > > +EXTRA_CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
>> > > -Wformat-nonliteral
>> > > +EXTRA_CFLAGS += -Wformat-security -Wundef -Wwrite-strings
>> > >
>> > >  CC     ?= gcc
>> > >  LD     ?= gcc
>> > > @@ -26,5 +26,5 @@ RMDIR  := rm -rf
>> > >  RM     := rm -f
>> > >  COPY   := cp -r
>> > >
>> > > -CFLAGS  += -pthread
>> > > +EXTRA_EXTRA_CFLAGS  += -pthread
>> > >  LDFLAGS += -pthread
>> > > diff --git a/platform/linux-generic/Makefile
>> > > b/platform/linux-generic/Makefile
>> > > index f665683..9828ee4 100644
>> > > --- a/platform/linux-generic/Makefile
>> > > +++ b/platform/linux-generic/Makefile
>> > > @@ -32,12 +32,12 @@ ODP_ROOT = ../..
>> > >  LIB_DIR  = ./lib
>> > >  DOC_DIR  = ./doc
>> > >
>> > > -CFLAGS  += -I$(ODP_ROOT)/include
>> > > -CFLAGS  += -I./include
>> > > -CFLAGS  += -I./include/api
>> > > +EXTRA_CFLAGS  += -I$(ODP_ROOT)/include
>> > > +EXTRA_CFLAGS  += -I./include
>> > > +EXTRA_CFLAGS  += -I./include/api
>> > >
>> > >  ifeq ($(ODP_HAVE_NETMAP),yes)
>> > > -CFLAGS  += -DODP_HAVE_NETMAP
>> > > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP
>> > >  endif
>> > >
>> > >  include $(ODP_ROOT)/Makefile.inc
>> > > @@ -92,7 +92,7 @@ $(DOC_DIR):
>> > >  #
>> > >  $(OBJ_DIR)/%.o: ./source/%.c
>> > >          $(ECHO) Compiling $<
>> > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>> > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>> > >
>> > >  #
>> > >  # Lib rule
>> > > diff --git a/test/api_test/Makefile b/test/api_test/Makefile
>> > > index 15c18f4..741fbe4 100644
>> > > --- a/test/api_test/Makefile
>> > > +++ b/test/api_test/Makefile
>> > > @@ -12,12 +12,12 @@ ODP_ATOMIC    = odp_atomic
>> > >  ODP_SHM       = odp_shm
>> > >  ODP_RING      = odp_ring
>> > >
>> > > -CFLAGS  += -I$(ODP_ROOT)/platform/linux-generic/include
>> > > +EXTRA_CFLAGS  += -I$(ODP_ROOT)/platform/linux-generic/include
>> > >
>> > >  include ../Makefile.inc
>> > >  include $(ODP_ROOT)/Makefile.inc
>> > >
>> > > -CFLAGS      += -I$(ODP_TEST_ROOT)/api_test
>> > > +EXTRA_CFLAGS      += -I$(ODP_TEST_ROOT)/api_test
>> > >
>> > >  ATOMIC_OBJS  =
>> > >  ATOMIC_OBJS += $(OBJ_DIR)/odp_common.o
>> > > @@ -52,7 +52,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>> > >  #
>> > >  $(OBJ_DIR)/%.o: %.c
>> > >          $(ECHO) Compiling $<
>> > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>> > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>> > >
>> > >  #
>> > >  # Link rule
>> > > diff --git a/test/example/Makefile b/test/example/Makefile
>> > > index d43e780..8064977 100644
>> > > --- a/test/example/Makefile
>> > > +++ b/test/example/Makefile
>> > > @@ -30,7 +30,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>> > >  #
>> > >  $(OBJ_DIR)/%.o: %.c
>> > >          $(ECHO) Compiling $<
>> > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>> > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>> > >
>> > >  #
>> > >  # Link rule
>> > > diff --git a/test/packet/Makefile b/test/packet/Makefile
>> > > index f1cb7d9..c66b37c 100644
>> > > --- a/test/packet/Makefile
>> > > +++ b/test/packet/Makefile
>> > > @@ -9,7 +9,7 @@ ODP_APP  = odp_packet
>> > >  include ../Makefile.inc
>> > >  include $(ODP_ROOT)/Makefile.inc
>> > >
>> > > -CFLAGS  += -I$(ODP_TEST_ROOT)/packet
>> > > +EXTRA_CFLAGS  += -I$(ODP_TEST_ROOT)/packet
>> > >
>> > >  OBJS     =
>> > >  OBJS    += $(OBJ_DIR)/odp_example_pktio.o
>> > > @@ -32,7 +32,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>> > >  #
>> > >  $(OBJ_DIR)/%.o: %.c
>> > >          $(ECHO) Compiling $<
>> > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>> > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>> > >
>> > >  #
>> > >  # Link rule
>> > > diff --git a/test/packet_netmap/Makefile b/test/packet_netmap/Makefile
>> > > index 5febb33..d92d113 100644
>> > > --- a/test/packet_netmap/Makefile
>> > > +++ b/test/packet_netmap/Makefile
>> > > @@ -6,8 +6,8 @@
>> > >  ODP_ROOT = ../..
>> > >  ODP_APP  = odp_packet
>> > >
>> > > -CFLAGS  += -DODP_HAVE_NETMAP
>> > > -CFLAGS  += -O0 -g
>> > > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP
>> > > +EXTRA_CFLAGS  += -O0 -g
>> > >
>> > >  include ../Makefile.inc
>> > >  include $(ODP_ROOT)/Makefile.inc
>> > > @@ -33,7 +33,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>> > >  #
>> > >  $(OBJ_DIR)/%.o: %.c
>> > >          $(ECHO) Compiling $<
>> > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>> > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>> > >
>> > >  #
>> > >  # Link rule
>> > > --
>> > > 1.8.3.2
>> > >
>> > >
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> Groups "LNG ODP Sub-team - lng-odp@linaro.org" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> an email to lng-odp+unsubscribe@linaro.org.
>> > To post to this group, send email to lng-odp@linaro.org.
>> > Visit this group at
>> http://groups.google.com/a/linaro.org/group/lng-odp/.
>> > To view this discussion on the web visit
>> https://groups.google.com/a/linaro.org/d/msgid/lng-odp/2b98325d-9086-4a5c-ada0-f5c7dfe81e17%40linaro.org
>> .
>> > For more options, visit
>> https://groups.google.com/a/linaro.org/groups/opt_out.
>>
>> Petri Mike,
>>
>> Would this solve the problems?
>>
>> For clang you need to specify: make CC=clang EXTRA_CFLAGS=-Wno-error
>> would that work?
>>
>> Drawback with this solution is that we can't use -O3 + ODP_DBG macro, do
>> we want that? if yes, then we just need to change:
>> ifeq ($(ODP_DEBUG), 1) to ifeq ($(a_good_name), 1)
>> ideas of a good name?
>>
>> diff --git a/Makefile.inc b/Makefile.inc
>> index 523385d..b35e030 100644
>> --- a/Makefile.inc
>> +++ b/Makefile.inc
>> @@ -4,17 +4,24 @@
>>  # SPDX-License-Identifier:     BSD-3-Clause
>>
>>  PLATFORM ?= linux-generic
>> -CFLAGS  += -DODP_DEBUG=1
>> -CFLAGS  += -O3
>> -#CFLAGS  += -O0 -g
>> +ODP_DEBUG ?= 0
>>
>>  OBJ_DIR  = ./obj
>>  DESTDIR ?= $(ODP_ROOT)/build
>> +EXTRA_CFLAGS ?=
>>
>>  CFLAGS += -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes
>>  CFLAGS += -Wmissing-declarations -Wold-style-definition -Wpointer-arith
>>  CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual -Wformat-nonliteral
>>  CFLAGS += -Wformat-security -Wundef -Wwrite-strings
>> +CFLAGS += $(EXTRA_CFLAGS)
>> +CFLAGS += -DODP_DEBUG=$(ODP_DEBUG)
>> +
>> +ifeq ($(ODP_DEBUG), 1)
>> +  CFLAGS  += -O0 -g
>> +else
>> +  CFLAGS  += -O3
>> +endif
>>
>>  CC     ?= gcc
>>  LD     ?= gcc
>>
>>
>> --
>> Anders Roxell
>> anders.roxell@linaro.org
>> M: +46 709 71 42 85 | IRC: roxell
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "LNG ODP Sub-team - lng-odp@linaro.org" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to lng-odp+unsubscribe@linaro.org.
>> To post to this group, send email to lng-odp@linaro.org.
>> Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/.
>> To view this discussion on the web visit
>> https://groups.google.com/a/linaro.org/d/msgid/lng-odp/20140218112207.GD3517%408470w
>> .
>> For more options, visit
>> https://groups.google.com/a/linaro.org/groups/opt_out.
>>
>
>
Maxim Uvarov Feb. 19, 2014, 1:27 p.m. UTC | #3
this patch needs some work.
For example:
make CFLAGS="-O2 -g"
is broken.

Maxim.

On 02/18/2014 10:37 PM, Mike Holmes wrote:
> Anders,
>
> Look up gnu Make recommendations, they suggest that a variable like 
> EXTRA_FLAGS be the internal variable with the defaults, and that 
> CFLAGS be the one a user would alter if they want to change the 
> optimization level etc from the default.
>
> I am not a fan of adding ODP_DEBUG, I think it is just as easy, if 
> I don't want the default to say
> "make CFLAGS=-O0 -g "
>
> That way we don't clutter our make with special cases, if we open the 
> dor to ODP_DEBUG,  we will start adding ODP_TRACING to add the gcov 
> and gcov flags and it is a slippery slope of special case from there.
>
>
> On 18 February 2014 09:43, Mike Holmes <mike.holmes@linaro.org 
> <mailto:mike.holmes@linaro.org>> wrote:
>
>     "As I said earlier, -O3 must be used by default. It must be used
>     in regression testing"
>     Nothing I propose alters the defaults, they just allow things to
>     be changed as needed, for regression we need to also compile with
>     debug on and this allow that permutation.
>
>
>     On 18 February 2014 06:22, Anders Roxell <anders.roxell@linaro.org
>     <mailto:anders.roxell@linaro.org>> wrote:
>
>         On 2014-02-17 23:55, Petri Savolainen wrote:
>         >
>         >
>         > On Tuesday, 18 February 2014 00:16:29 UTC+2, Mike Holmes wrote:
>         > >
>         > > Signed-off-by: Mike Holmes <mike....@linaro.org
>         <mailto:mike....@linaro.org> <javascript:>>
>         > > ---
>         > >
>         > > CFLAGS is usually expected to take effect on a build if
>         applied from the
>         > > command line when calling make, or if it is set in the
>         environment.
>         > >
>         > > Examples are when building with CLANG, -Wno-error must be
>         applied to
>         > > allow CLANG warnings which are a superset of gcc to cause
>         the build to
>         > > fail. e.g. make CC=clang CFLAGS=-Wno-error
>         > >
>         > > When building with optimizations off and with -g for
>         debugging, it must
>         > > be possible to alter the defaults, in ODPs case these
>         defaults are in
>         > > Makefile.inc
>         > >
>         > > e.g. make CFLAGS=-O3
>         > >
>         > >  .checkpatch.conf                |  1 +
>         > >  Makefile.inc                    | 16 ++++++++--------
>         > >  platform/linux-generic/Makefile | 10 +++++-----
>         > >  test/api_test/Makefile          |  6 +++---
>         > >  test/example/Makefile           |  2 +-
>         > >  test/packet/Makefile            |  4 ++--
>         > >  test/packet_netmap/Makefile     |  6 +++---
>         > >  7 files changed, 23 insertions(+), 22 deletions(-)
>         > >
>         > > diff --git a/.checkpatch.conf b/.checkpatch.conf
>         > > index e1a25c8..9076410 100644
>         > > --- a/.checkpatch.conf
>         > > +++ b/.checkpatch.conf
>         > > @@ -1,3 +1,4 @@
>         > >  --no-tree
>         > >  --strict
>         > >  --ignore=NEW_TYPEDEFS
>         > > +--ignore=DEPRECATED_VARIABLE
>         > > diff --git a/Makefile.inc b/Makefile.inc
>         > > index 7dea028..75a4829 100644
>         > > --- a/Makefile.inc
>         > > +++ b/Makefile.inc
>         > > @@ -4,17 +4,17 @@
>         > >  # SPDX-License-Identifier:  BSD-3-Clause
>         > >
>         > >  PLATFORM ?= linux-generic
>         > > -CFLAGS  += -DODP_DEBUG=1
>         > > -#CFLAGS  += -O3
>         > > -CFLAGS  += -O0 -g
>         > > +EXTRA_CFLAGS  += -DODP_DEBUG=1
>         > > +#EXTRA_CFLAGS  += -O3
>         > > +EXTRA_CFLAGS  += -O0 -g
>         > >
>         >
>         > As I said earlier, -O3 must be used by default. It must be
>         used in
>         > regression testing. Otherwise we have code soon that does
>         not compile with
>         > -O3, or compiles but has bugs with optimization.
>         >
>         > -Petri
>         >
>         >
>         > >
>         > >  OBJ_DIR  = ./obj
>         > >  DESTDIR ?= $(ODP_ROOT)/build
>         > >
>         > > -CFLAGS += -W -Wall -Werror -Wstrict-prototypes
>         -Wmissing-prototypes
>         > > -CFLAGS += -Wmissing-declarations -Wold-style-definition
>         -Wpointer-arith
>         > > -CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
>         -Wformat-nonliteral
>         > > -CFLAGS += -Wformat-security -Wundef -Wwrite-strings
>         > > +EXTRA_CFLAGS += -W -Wall -Werror -Wstrict-prototypes
>         -Wmissing-prototypes
>         > > +EXTRA_CFLAGS += -Wmissing-declarations -Wold-style-definition
>         > > -Wpointer-arith
>         > > +EXTRA_CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
>         > > -Wformat-nonliteral
>         > > +EXTRA_CFLAGS += -Wformat-security -Wundef -Wwrite-strings
>         > >
>         > >  CC     ?= gcc
>         > >  LD     ?= gcc
>         > > @@ -26,5 +26,5 @@ RMDIR  := rm -rf
>         > >  RM     := rm -f
>         > >  COPY   := cp -r
>         > >
>         > > -CFLAGS  += -pthread
>         > > +EXTRA_EXTRA_CFLAGS  += -pthread
>         > >  LDFLAGS += -pthread
>         > > diff --git a/platform/linux-generic/Makefile
>         > > b/platform/linux-generic/Makefile
>         > > index f665683..9828ee4 100644
>         > > --- a/platform/linux-generic/Makefile
>         > > +++ b/platform/linux-generic/Makefile
>         > > @@ -32,12 +32,12 @@ ODP_ROOT = ../..
>         > >  LIB_DIR  = ./lib
>         > >  DOC_DIR  = ./doc
>         > >
>         > > -CFLAGS  += -I$(ODP_ROOT)/include
>         > > -CFLAGS  += -I./include
>         > > -CFLAGS  += -I./include/api
>         > > +EXTRA_CFLAGS  += -I$(ODP_ROOT)/include
>         > > +EXTRA_CFLAGS  += -I./include
>         > > +EXTRA_CFLAGS  += -I./include/api
>         > >
>         > >  ifeq ($(ODP_HAVE_NETMAP),yes)
>         > > -CFLAGS  += -DODP_HAVE_NETMAP
>         > > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP
>         > >  endif
>         > >
>         > >  include $(ODP_ROOT)/Makefile.inc
>         > > @@ -92,7 +92,7 @@ $(DOC_DIR):
>         > >  #
>         > >  $(OBJ_DIR)/%.o: ./source/%.c
>         > >          $(ECHO) Compiling $<
>         > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>         > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>         > >
>         > >  #
>         > >  # Lib rule
>         > > diff --git a/test/api_test/Makefile b/test/api_test/Makefile
>         > > index 15c18f4..741fbe4 100644
>         > > --- a/test/api_test/Makefile
>         > > +++ b/test/api_test/Makefile
>         > > @@ -12,12 +12,12 @@ ODP_ATOMIC    = odp_atomic
>         > >  ODP_SHM       = odp_shm
>         > >  ODP_RING      = odp_ring
>         > >
>         > > -CFLAGS  += -I$(ODP_ROOT)/platform/linux-generic/include
>         > > +EXTRA_CFLAGS  += -I$(ODP_ROOT)/platform/linux-generic/include
>         > >
>         > >  include ../Makefile.inc
>         > >  include $(ODP_ROOT)/Makefile.inc
>         > >
>         > > -CFLAGS      += -I$(ODP_TEST_ROOT)/api_test
>         > > +EXTRA_CFLAGS      += -I$(ODP_TEST_ROOT)/api_test
>         > >
>         > >  ATOMIC_OBJS  =
>         > >  ATOMIC_OBJS += $(OBJ_DIR)/odp_common.o
>         > > @@ -52,7 +52,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>         > >  #
>         > >  $(OBJ_DIR)/%.o: %.c
>         > >          $(ECHO) Compiling $<
>         > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>         > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>         > >
>         > >  #
>         > >  # Link rule
>         > > diff --git a/test/example/Makefile b/test/example/Makefile
>         > > index d43e780..8064977 100644
>         > > --- a/test/example/Makefile
>         > > +++ b/test/example/Makefile
>         > > @@ -30,7 +30,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>         > >  #
>         > >  $(OBJ_DIR)/%.o: %.c
>         > >          $(ECHO) Compiling $<
>         > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>         > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>         > >
>         > >  #
>         > >  # Link rule
>         > > diff --git a/test/packet/Makefile b/test/packet/Makefile
>         > > index f1cb7d9..c66b37c 100644
>         > > --- a/test/packet/Makefile
>         > > +++ b/test/packet/Makefile
>         > > @@ -9,7 +9,7 @@ ODP_APP  = odp_packet
>         > >  include ../Makefile.inc
>         > >  include $(ODP_ROOT)/Makefile.inc
>         > >
>         > > -CFLAGS  += -I$(ODP_TEST_ROOT)/packet
>         > > +EXTRA_CFLAGS  += -I$(ODP_TEST_ROOT)/packet
>         > >
>         > >  OBJS     =
>         > >  OBJS    += $(OBJ_DIR)/odp_example_pktio.o
>         > > @@ -32,7 +32,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>         > >  #
>         > >  $(OBJ_DIR)/%.o: %.c
>         > >          $(ECHO) Compiling $<
>         > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>         > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>         > >
>         > >  #
>         > >  # Link rule
>         > > diff --git a/test/packet_netmap/Makefile
>         b/test/packet_netmap/Makefile
>         > > index 5febb33..d92d113 100644
>         > > --- a/test/packet_netmap/Makefile
>         > > +++ b/test/packet_netmap/Makefile
>         > > @@ -6,8 +6,8 @@
>         > >  ODP_ROOT = ../..
>         > >  ODP_APP  = odp_packet
>         > >
>         > > -CFLAGS  += -DODP_HAVE_NETMAP
>         > > -CFLAGS  += -O0 -g
>         > > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP
>         > > +EXTRA_CFLAGS  += -O0 -g
>         > >
>         > >  include ../Makefile.inc
>         > >  include $(ODP_ROOT)/Makefile.inc
>         > > @@ -33,7 +33,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>         > >  #
>         > >  $(OBJ_DIR)/%.o: %.c
>         > >          $(ECHO) Compiling $<
>         > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>         > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>         > >
>         > >  #
>         > >  # Link rule
>         > > --
>         > > 1.8.3.2
>         > >
>         > >
>         >
>         > --
>         > You received this message because you are subscribed to the
>         Google Groups "LNG ODP Sub-team - lng-odp@linaro.org
>         <mailto:lng-odp@linaro.org>" group.
>         > To unsubscribe from this group and stop receiving emails
>         from it, send an email to lng-odp+unsubscribe@linaro.org
>         <mailto:lng-odp%2Bunsubscribe@linaro.org>.
>         > To post to this group, send email to lng-odp@linaro.org
>         <mailto:lng-odp@linaro.org>.
>         > Visit this group at
>         http://groups.google.com/a/linaro.org/group/lng-odp/.
>         > To view this discussion on the web visit
>         https://groups.google.com/a/linaro.org/d/msgid/lng-odp/2b98325d-9086-4a5c-ada0-f5c7dfe81e17%40linaro.org.
>         > For more options, visit
>         https://groups.google.com/a/linaro.org/groups/opt_out.
>
>         Petri Mike,
>
>         Would this solve the problems?
>
>         For clang you need to specify: make CC=clang
>         EXTRA_CFLAGS=-Wno-error
>         would that work?
>
>         Drawback with this solution is that we can't use -O3 + ODP_DBG
>         macro, do
>         we want that? if yes, then we just need to change:
>         ifeq ($(ODP_DEBUG), 1) to ifeq ($(a_good_name), 1)
>         ideas of a good name?
>
>         diff --git a/Makefile.inc b/Makefile.inc
>         index 523385d..b35e030 100644
>         --- a/Makefile.inc
>         +++ b/Makefile.inc
>         @@ -4,17 +4,24 @@
>          # SPDX-License-Identifier:     BSD-3-Clause
>
>          PLATFORM ?= linux-generic
>         -CFLAGS  += -DODP_DEBUG=1
>         -CFLAGS  += -O3
>         -#CFLAGS  += -O0 -g
>         +ODP_DEBUG ?= 0
>
>          OBJ_DIR  = ./obj
>          DESTDIR ?= $(ODP_ROOT)/build
>         +EXTRA_CFLAGS ?=
>
>          CFLAGS += -W -Wall -Werror -Wstrict-prototypes
>         -Wmissing-prototypes
>          CFLAGS += -Wmissing-declarations -Wold-style-definition
>         -Wpointer-arith
>          CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
>         -Wformat-nonliteral
>          CFLAGS += -Wformat-security -Wundef -Wwrite-strings
>         +CFLAGS += $(EXTRA_CFLAGS)
>         +CFLAGS += -DODP_DEBUG=$(ODP_DEBUG)
>         +
>         +ifeq ($(ODP_DEBUG), 1)
>         +  CFLAGS  += -O0 -g
>         +else
>         +  CFLAGS  += -O3
>         +endif
>
>          CC     ?= gcc
>          LD     ?= gcc
>
>
>         --
>         Anders Roxell
>         anders.roxell@linaro.org <mailto:anders.roxell@linaro.org>
>         M: +46 709 71 42 85 <tel:%2B46%20709%2071%2042%2085> | IRC: roxell
>
>         --
>         You received this message because you are subscribed to the
>         Google Groups "LNG ODP Sub-team - lng-odp@linaro.org
>         <mailto:lng-odp@linaro.org>" group.
>         To unsubscribe from this group and stop receiving emails from
>         it, send an email to lng-odp+unsubscribe@linaro.org
>         <mailto:lng-odp%2Bunsubscribe@linaro.org>.
>         To post to this group, send email to lng-odp@linaro.org
>         <mailto:lng-odp@linaro.org>.
>         Visit this group at
>         http://groups.google.com/a/linaro.org/group/lng-odp/.
>         To view this discussion on the web visit
>         https://groups.google.com/a/linaro.org/d/msgid/lng-odp/20140218112207.GD3517%408470w.
>         For more options, visit
>         https://groups.google.com/a/linaro.org/groups/opt_out.
>
>
>
> -- 
> You received this message because you are subscribed to the Google 
> Groups "LNG ODP Sub-team - lng-odp@linaro.org" group.
> To unsubscribe from this group and stop receiving emails from it, send 
> an email to lng-odp+unsubscribe@linaro.org.
> To post to this group, send email to lng-odp@linaro.org.
> Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/.
> To view this discussion on the web visit 
> https://groups.google.com/a/linaro.org/d/msgid/lng-odp/CADz3at18gL9DJ8kUKq4WNft0c4mHutEJ49Mj2ZXFULULpPA34w%40mail.gmail.com.
> For more options, visit 
> https://groups.google.com/a/linaro.org/groups/opt_out.
Mike Holmes Feb. 19, 2014, 4:12 p.m. UTC | #4
Don't apply Anders changes, we do want the original patch however.


On 19 February 2014 08:27, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:

> this patch needs some work.
> For example:
> make CFLAGS="-O2 -g"
> is broken.
>
> Maxim.
>
>
> On 02/18/2014 10:37 PM, Mike Holmes wrote:
>
>> Anders,
>>
>> Look up gnu Make recommendations, they suggest that a variable like
>> EXTRA_FLAGS be the internal variable with the defaults, and that CFLAGS be
>> the one a user would alter if they want to change the optimization level
>> etc from the default.
>>
>> I am not a fan of adding ODP_DEBUG, I think it is just as easy, if I
>> don't want the default to say
>> "make CFLAGS=-O0 -g "
>>
>> That way we don't clutter our make with special cases, if we open the dor
>> to ODP_DEBUG,  we will start adding ODP_TRACING to add the gcov and gcov
>> flags and it is a slippery slope of special case from there.
>>
>>
>> On 18 February 2014 09:43, Mike Holmes <mike.holmes@linaro.org <mailto:
>> mike.holmes@linaro.org>> wrote:
>>
>>     "As I said earlier, -O3 must be used by default. It must be used
>>     in regression testing"
>>     Nothing I propose alters the defaults, they just allow things to
>>     be changed as needed, for regression we need to also compile with
>>     debug on and this allow that permutation.
>>
>>
>>     On 18 February 2014 06:22, Anders Roxell <anders.roxell@linaro.org
>>     <mailto:anders.roxell@linaro.org>> wrote:
>>
>>         On 2014-02-17 23:55, Petri Savolainen wrote:
>>         >
>>         >
>>         > On Tuesday, 18 February 2014 00:16:29 UTC+2, Mike Holmes wrote:
>>         > >
>>         > > Signed-off-by: Mike Holmes <mike....@linaro.org
>>         <mailto:mike....@linaro.org> <javascript:>>
>>
>>         > > ---
>>         > >
>>         > > CFLAGS is usually expected to take effect on a build if
>>         applied from the
>>         > > command line when calling make, or if it is set in the
>>         environment.
>>         > >
>>         > > Examples are when building with CLANG, -Wno-error must be
>>         applied to
>>         > > allow CLANG warnings which are a superset of gcc to cause
>>         the build to
>>         > > fail. e.g. make CC=clang CFLAGS=-Wno-error
>>         > >
>>         > > When building with optimizations off and with -g for
>>         debugging, it must
>>         > > be possible to alter the defaults, in ODPs case these
>>         defaults are in
>>         > > Makefile.inc
>>         > >
>>         > > e.g. make CFLAGS=-O3
>>         > >
>>         > >  .checkpatch.conf                |  1 +
>>         > >  Makefile.inc                    | 16 ++++++++--------
>>         > >  platform/linux-generic/Makefile | 10 +++++-----
>>         > >  test/api_test/Makefile          |  6 +++---
>>         > >  test/example/Makefile           |  2 +-
>>         > >  test/packet/Makefile            |  4 ++--
>>         > >  test/packet_netmap/Makefile     |  6 +++---
>>         > >  7 files changed, 23 insertions(+), 22 deletions(-)
>>         > >
>>         > > diff --git a/.checkpatch.conf b/.checkpatch.conf
>>         > > index e1a25c8..9076410 100644
>>         > > --- a/.checkpatch.conf
>>         > > +++ b/.checkpatch.conf
>>         > > @@ -1,3 +1,4 @@
>>         > >  --no-tree
>>         > >  --strict
>>         > >  --ignore=NEW_TYPEDEFS
>>         > > +--ignore=DEPRECATED_VARIABLE
>>         > > diff --git a/Makefile.inc b/Makefile.inc
>>         > > index 7dea028..75a4829 100644
>>         > > --- a/Makefile.inc
>>         > > +++ b/Makefile.inc
>>         > > @@ -4,17 +4,17 @@
>>         > >  # SPDX-License-Identifier:  BSD-3-Clause
>>         > >
>>         > >  PLATFORM ?= linux-generic
>>         > > -CFLAGS  += -DODP_DEBUG=1
>>         > > -#CFLAGS  += -O3
>>         > > -CFLAGS  += -O0 -g
>>         > > +EXTRA_CFLAGS  += -DODP_DEBUG=1
>>         > > +#EXTRA_CFLAGS  += -O3
>>         > > +EXTRA_CFLAGS  += -O0 -g
>>         > >
>>         >
>>         > As I said earlier, -O3 must be used by default. It must be
>>         used in
>>         > regression testing. Otherwise we have code soon that does
>>         not compile with
>>         > -O3, or compiles but has bugs with optimization.
>>         >
>>         > -Petri
>>         >
>>         >
>>         > >
>>         > >  OBJ_DIR  = ./obj
>>         > >  DESTDIR ?= $(ODP_ROOT)/build
>>         > >
>>         > > -CFLAGS += -W -Wall -Werror -Wstrict-prototypes
>>         -Wmissing-prototypes
>>         > > -CFLAGS += -Wmissing-declarations -Wold-style-definition
>>         -Wpointer-arith
>>         > > -CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
>>         -Wformat-nonliteral
>>         > > -CFLAGS += -Wformat-security -Wundef -Wwrite-strings
>>         > > +EXTRA_CFLAGS += -W -Wall -Werror -Wstrict-prototypes
>>         -Wmissing-prototypes
>>         > > +EXTRA_CFLAGS += -Wmissing-declarations -Wold-style-definition
>>         > > -Wpointer-arith
>>         > > +EXTRA_CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
>>         > > -Wformat-nonliteral
>>         > > +EXTRA_CFLAGS += -Wformat-security -Wundef -Wwrite-strings
>>         > >
>>         > >  CC     ?= gcc
>>         > >  LD     ?= gcc
>>         > > @@ -26,5 +26,5 @@ RMDIR  := rm -rf
>>         > >  RM     := rm -f
>>         > >  COPY   := cp -r
>>         > >
>>         > > -CFLAGS  += -pthread
>>         > > +EXTRA_EXTRA_CFLAGS  += -pthread
>>         > >  LDFLAGS += -pthread
>>         > > diff --git a/platform/linux-generic/Makefile
>>         > > b/platform/linux-generic/Makefile
>>         > > index f665683..9828ee4 100644
>>         > > --- a/platform/linux-generic/Makefile
>>         > > +++ b/platform/linux-generic/Makefile
>>         > > @@ -32,12 +32,12 @@ ODP_ROOT = ../..
>>         > >  LIB_DIR  = ./lib
>>         > >  DOC_DIR  = ./doc
>>         > >
>>         > > -CFLAGS  += -I$(ODP_ROOT)/include
>>         > > -CFLAGS  += -I./include
>>         > > -CFLAGS  += -I./include/api
>>         > > +EXTRA_CFLAGS  += -I$(ODP_ROOT)/include
>>         > > +EXTRA_CFLAGS  += -I./include
>>         > > +EXTRA_CFLAGS  += -I./include/api
>>         > >
>>         > >  ifeq ($(ODP_HAVE_NETMAP),yes)
>>         > > -CFLAGS  += -DODP_HAVE_NETMAP
>>         > > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP
>>         > >  endif
>>         > >
>>         > >  include $(ODP_ROOT)/Makefile.inc
>>         > > @@ -92,7 +92,7 @@ $(DOC_DIR):
>>         > >  #
>>         > >  $(OBJ_DIR)/%.o: ./source/%.c
>>         > >          $(ECHO) Compiling $<
>>         > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>>         > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>>         > >
>>         > >  #
>>         > >  # Lib rule
>>         > > diff --git a/test/api_test/Makefile b/test/api_test/Makefile
>>         > > index 15c18f4..741fbe4 100644
>>         > > --- a/test/api_test/Makefile
>>         > > +++ b/test/api_test/Makefile
>>         > > @@ -12,12 +12,12 @@ ODP_ATOMIC    = odp_atomic
>>         > >  ODP_SHM       = odp_shm
>>         > >  ODP_RING      = odp_ring
>>         > >
>>         > > -CFLAGS  += -I$(ODP_ROOT)/platform/linux-generic/include
>>         > > +EXTRA_CFLAGS  += -I$(ODP_ROOT)/platform/linux-
>> generic/include
>>         > >
>>         > >  include ../Makefile.inc
>>         > >  include $(ODP_ROOT)/Makefile.inc
>>         > >
>>         > > -CFLAGS      += -I$(ODP_TEST_ROOT)/api_test
>>         > > +EXTRA_CFLAGS      += -I$(ODP_TEST_ROOT)/api_test
>>         > >
>>         > >  ATOMIC_OBJS  =
>>         > >  ATOMIC_OBJS += $(OBJ_DIR)/odp_common.o
>>         > > @@ -52,7 +52,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>>         > >  #
>>         > >  $(OBJ_DIR)/%.o: %.c
>>         > >          $(ECHO) Compiling $<
>>         > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>>         > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>>         > >
>>         > >  #
>>         > >  # Link rule
>>         > > diff --git a/test/example/Makefile b/test/example/Makefile
>>         > > index d43e780..8064977 100644
>>         > > --- a/test/example/Makefile
>>         > > +++ b/test/example/Makefile
>>         > > @@ -30,7 +30,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>>         > >  #
>>         > >  $(OBJ_DIR)/%.o: %.c
>>         > >          $(ECHO) Compiling $<
>>         > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>>         > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>>         > >
>>         > >  #
>>         > >  # Link rule
>>         > > diff --git a/test/packet/Makefile b/test/packet/Makefile
>>         > > index f1cb7d9..c66b37c 100644
>>         > > --- a/test/packet/Makefile
>>         > > +++ b/test/packet/Makefile
>>         > > @@ -9,7 +9,7 @@ ODP_APP  = odp_packet
>>         > >  include ../Makefile.inc
>>         > >  include $(ODP_ROOT)/Makefile.inc
>>         > >
>>         > > -CFLAGS  += -I$(ODP_TEST_ROOT)/packet
>>         > > +EXTRA_CFLAGS  += -I$(ODP_TEST_ROOT)/packet
>>         > >
>>         > >  OBJS     =
>>         > >  OBJS    += $(OBJ_DIR)/odp_example_pktio.o
>>         > > @@ -32,7 +32,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>>         > >  #
>>         > >  $(OBJ_DIR)/%.o: %.c
>>         > >          $(ECHO) Compiling $<
>>         > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>>         > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>>         > >
>>         > >  #
>>         > >  # Link rule
>>         > > diff --git a/test/packet_netmap/Makefile
>>         b/test/packet_netmap/Makefile
>>         > > index 5febb33..d92d113 100644
>>         > > --- a/test/packet_netmap/Makefile
>>         > > +++ b/test/packet_netmap/Makefile
>>         > > @@ -6,8 +6,8 @@
>>         > >  ODP_ROOT = ../..
>>         > >  ODP_APP  = odp_packet
>>         > >
>>         > > -CFLAGS  += -DODP_HAVE_NETMAP
>>         > > -CFLAGS  += -O0 -g
>>         > > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP
>>         > > +EXTRA_CFLAGS  += -O0 -g
>>         > >
>>         > >  include ../Makefile.inc
>>         > >  include $(ODP_ROOT)/Makefile.inc
>>         > > @@ -33,7 +33,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>>         > >  #
>>         > >  $(OBJ_DIR)/%.o: %.c
>>         > >          $(ECHO) Compiling $<
>>         > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>>         > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o $@ $<
>>         > >
>>         > >  #
>>         > >  # Link rule
>>         > > --
>>         > > 1.8.3.2
>>         > >
>>         > >
>>         >
>>         > --
>>         > You received this message because you are subscribed to the
>>         Google Groups "LNG ODP Sub-team - lng-odp@linaro.org
>>         <mailto:lng-odp@linaro.org>" group.
>>
>>         > To unsubscribe from this group and stop receiving emails
>>         from it, send an email to lng-odp+unsubscribe@linaro.org
>>         <mailto:lng-odp%2Bunsubscribe@linaro.org>.
>>
>>         > To post to this group, send email to lng-odp@linaro.org
>>         <mailto:lng-odp@linaro.org>.
>>
>>         > Visit this group at
>>         http://groups.google.com/a/linaro.org/group/lng-odp/.
>>         > To view this discussion on the web visit
>>         https://groups.google.com/a/linaro.org/d/msgid/lng-odp/
>> 2b98325d-9086-4a5c-ada0-f5c7dfe81e17%40linaro.org.
>>         > For more options, visit
>>         https://groups.google.com/a/linaro.org/groups/opt_out.
>>
>>         Petri Mike,
>>
>>         Would this solve the problems?
>>
>>         For clang you need to specify: make CC=clang
>>         EXTRA_CFLAGS=-Wno-error
>>         would that work?
>>
>>         Drawback with this solution is that we can't use -O3 + ODP_DBG
>>         macro, do
>>         we want that? if yes, then we just need to change:
>>         ifeq ($(ODP_DEBUG), 1) to ifeq ($(a_good_name), 1)
>>         ideas of a good name?
>>
>>         diff --git a/Makefile.inc b/Makefile.inc
>>         index 523385d..b35e030 100644
>>         --- a/Makefile.inc
>>         +++ b/Makefile.inc
>>         @@ -4,17 +4,24 @@
>>          # SPDX-License-Identifier:     BSD-3-Clause
>>
>>          PLATFORM ?= linux-generic
>>         -CFLAGS  += -DODP_DEBUG=1
>>         -CFLAGS  += -O3
>>         -#CFLAGS  += -O0 -g
>>         +ODP_DEBUG ?= 0
>>
>>          OBJ_DIR  = ./obj
>>          DESTDIR ?= $(ODP_ROOT)/build
>>         +EXTRA_CFLAGS ?=
>>
>>          CFLAGS += -W -Wall -Werror -Wstrict-prototypes
>>         -Wmissing-prototypes
>>          CFLAGS += -Wmissing-declarations -Wold-style-definition
>>         -Wpointer-arith
>>          CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
>>         -Wformat-nonliteral
>>          CFLAGS += -Wformat-security -Wundef -Wwrite-strings
>>         +CFLAGS += $(EXTRA_CFLAGS)
>>         +CFLAGS += -DODP_DEBUG=$(ODP_DEBUG)
>>         +
>>         +ifeq ($(ODP_DEBUG), 1)
>>         +  CFLAGS  += -O0 -g
>>         +else
>>         +  CFLAGS  += -O3
>>         +endif
>>
>>          CC     ?= gcc
>>          LD     ?= gcc
>>
>>
>>         --
>>         Anders Roxell
>>         anders.roxell@linaro.org <mailto:anders.roxell@linaro.org>
>>         M: +46 709 71 42 85 <tel:%2B46%20709%2071%2042%2085> | IRC:
>> roxell
>>
>>
>>         --
>>         You received this message because you are subscribed to the
>>         Google Groups "LNG ODP Sub-team - lng-odp@linaro.org
>>         <mailto:lng-odp@linaro.org>" group.
>>
>>         To unsubscribe from this group and stop receiving emails from
>>         it, send an email to lng-odp+unsubscribe@linaro.org
>>         <mailto:lng-odp%2Bunsubscribe@linaro.org>.
>>
>>         To post to this group, send email to lng-odp@linaro.org
>>         <mailto:lng-odp@linaro.org>.
>>
>>         Visit this group at
>>         http://groups.google.com/a/linaro.org/group/lng-odp/.
>>         To view this discussion on the web visit
>>         https://groups.google.com/a/linaro.org/d/msgid/lng-odp/
>> 20140218112207.GD3517%408470w.
>>         For more options, visit
>>         https://groups.google.com/a/linaro.org/groups/opt_out.
>>
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "LNG ODP Sub-team - lng-odp@linaro.org" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to lng-odp+unsubscribe@linaro.org.
>> To post to this group, send email to lng-odp@linaro.org.
>> Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/.
>> To view this discussion on the web visit https://groups.google.com/a/
>> linaro.org/d/msgid/lng-odp/CADz3at18gL9DJ8kUKq4WNft0c4mHu
>> tEJ49Mj2ZXFULULpPA34w%40mail.gmail.com.
>>
>> For more options, visit https://groups.google.com/a/
>> linaro.org/groups/opt_out.
>>
>
> --
> You received this message because you are subscribed to the Google Groups
> "LNG ODP Sub-team - lng-odp@linaro.org" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to lng-odp+unsubscribe@linaro.org.
> To post to this group, send email to lng-odp@linaro.org.
> Visit this group at http://groups.google.com/a/linaro.org/group/lng-odp/.
> To view this discussion on the web visit https://groups.google.com/a/
> linaro.org/d/msgid/lng-odp/5304B149.5060304%40linaro.org.
>
> For more options, visit https://groups.google.com/a/
> linaro.org/groups/opt_out.
>
Maxim Uvarov Feb. 19, 2014, 4:13 p.m. UTC | #5
On 02/19/2014 08:12 PM, Mike Holmes wrote:
> Don't apply Anders changes, we do want the original patch however.
>
I think I can apply your patch + my fix in one shot.

Maxim.
>
> On 19 February 2014 08:27, Maxim Uvarov <maxim.uvarov@linaro.org 
> <mailto:maxim.uvarov@linaro.org>> wrote:
>
>     this patch needs some work.
>     For example:
>     make CFLAGS="-O2 -g"
>     is broken.
>
>     Maxim.
>
>
>     On 02/18/2014 10:37 PM, Mike Holmes wrote:
>
>         Anders,
>
>         Look up gnu Make recommendations, they suggest that a variable
>         like EXTRA_FLAGS be the internal variable with the defaults,
>         and that CFLAGS be the one a user would alter if they want to
>         change the optimization level etc from the default.
>
>         I am not a fan of adding ODP_DEBUG, I think it is just as
>         easy, if I don't want the default to say
>         "make CFLAGS=-O0 -g "
>
>         That way we don't clutter our make with special cases, if we
>         open the dor to ODP_DEBUG,  we will start adding ODP_TRACING
>         to add the gcov and gcov flags and it is a slippery slope of
>         special case from there.
>
>
>         On 18 February 2014 09:43, Mike Holmes <mike.holmes@linaro.org
>         <mailto:mike.holmes@linaro.org> <mailto:mike.holmes@linaro.org
>         <mailto:mike.holmes@linaro.org>>> wrote:
>
>             "As I said earlier, -O3 must be used by default. It must
>         be used
>             in regression testing"
>             Nothing I propose alters the defaults, they just allow
>         things to
>             be changed as needed, for regression we need to also
>         compile with
>             debug on and this allow that permutation.
>
>
>             On 18 February 2014 06:22, Anders Roxell
>         <anders.roxell@linaro.org <mailto:anders.roxell@linaro.org>
>             <mailto:anders.roxell@linaro.org
>         <mailto:anders.roxell@linaro.org>>> wrote:
>
>                 On 2014-02-17 23:55, Petri Savolainen wrote:
>                 >
>                 >
>                 > On Tuesday, 18 February 2014 00:16:29 UTC+2, Mike
>         Holmes wrote:
>                 > >
>                 > > Signed-off-by: Mike Holmes <mike....@linaro.org
>         <mailto:mike....@linaro.org>
>                 <mailto:mike....@linaro.org
>         <mailto:mike....@linaro.org>> <javascript:>>
>
>                 > > ---
>                 > >
>                 > > CFLAGS is usually expected to take effect on a
>         build if
>                 applied from the
>                 > > command line when calling make, or if it is set in the
>                 environment.
>                 > >
>                 > > Examples are when building with CLANG, -Wno-error
>         must be
>                 applied to
>                 > > allow CLANG warnings which are a superset of gcc
>         to cause
>                 the build to
>                 > > fail. e.g. make CC=clang CFLAGS=-Wno-error
>                 > >
>                 > > When building with optimizations off and with -g for
>                 debugging, it must
>                 > > be possible to alter the defaults, in ODPs case these
>                 defaults are in
>                 > > Makefile.inc
>                 > >
>                 > > e.g. make CFLAGS=-O3
>                 > >
>                 > >  .checkpatch.conf                |  1 +
>                 > >  Makefile.inc                    | 16 ++++++++--------
>                 > >  platform/linux-generic/Makefile | 10 +++++-----
>                 > >  test/api_test/Makefile          |  6 +++---
>                 > >  test/example/Makefile           |  2 +-
>                 > >  test/packet/Makefile            |  4 ++--
>                 > >  test/packet_netmap/Makefile     |  6 +++---
>                 > >  7 files changed, 23 insertions(+), 22 deletions(-)
>                 > >
>                 > > diff --git a/.checkpatch.conf b/.checkpatch.conf
>                 > > index e1a25c8..9076410 100644
>                 > > --- a/.checkpatch.conf
>                 > > +++ b/.checkpatch.conf
>                 > > @@ -1,3 +1,4 @@
>                 > >  --no-tree
>                 > >  --strict
>                 > >  --ignore=NEW_TYPEDEFS
>                 > > +--ignore=DEPRECATED_VARIABLE
>                 > > diff --git a/Makefile.inc b/Makefile.inc
>                 > > index 7dea028..75a4829 100644
>                 > > --- a/Makefile.inc
>                 > > +++ b/Makefile.inc
>                 > > @@ -4,17 +4,17 @@
>                 > >  # SPDX-License-Identifier:  BSD-3-Clause
>                 > >
>                 > >  PLATFORM ?= linux-generic
>                 > > -CFLAGS  += -DODP_DEBUG=1
>                 > > -#CFLAGS  += -O3
>                 > > -CFLAGS  += -O0 -g
>                 > > +EXTRA_CFLAGS  += -DODP_DEBUG=1
>                 > > +#EXTRA_CFLAGS  += -O3
>                 > > +EXTRA_CFLAGS  += -O0 -g
>                 > >
>                 >
>                 > As I said earlier, -O3 must be used by default. It
>         must be
>                 used in
>                 > regression testing. Otherwise we have code soon that
>         does
>                 not compile with
>                 > -O3, or compiles but has bugs with optimization.
>                 >
>                 > -Petri
>                 >
>                 >
>                 > >
>                 > >  OBJ_DIR  = ./obj
>                 > >  DESTDIR ?= $(ODP_ROOT)/build
>                 > >
>                 > > -CFLAGS += -W -Wall -Werror -Wstrict-prototypes
>                 -Wmissing-prototypes
>                 > > -CFLAGS += -Wmissing-declarations
>         -Wold-style-definition
>                 -Wpointer-arith
>                 > > -CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
>                 -Wformat-nonliteral
>                 > > -CFLAGS += -Wformat-security -Wundef -Wwrite-strings
>                 > > +EXTRA_CFLAGS += -W -Wall -Werror -Wstrict-prototypes
>                 -Wmissing-prototypes
>                 > > +EXTRA_CFLAGS += -Wmissing-declarations
>         -Wold-style-definition
>                 > > -Wpointer-arith
>                 > > +EXTRA_CFLAGS += -Wcast-align -Wnested-externs
>         -Wcast-qual
>                 > > -Wformat-nonliteral
>                 > > +EXTRA_CFLAGS += -Wformat-security -Wundef
>         -Wwrite-strings
>                 > >
>                 > >  CC     ?= gcc
>                 > >  LD     ?= gcc
>                 > > @@ -26,5 +26,5 @@ RMDIR  := rm -rf
>                 > >  RM     := rm -f
>                 > >  COPY   := cp -r
>                 > >
>                 > > -CFLAGS  += -pthread
>                 > > +EXTRA_EXTRA_CFLAGS  += -pthread
>                 > >  LDFLAGS += -pthread
>                 > > diff --git a/platform/linux-generic/Makefile
>                 > > b/platform/linux-generic/Makefile
>                 > > index f665683..9828ee4 100644
>                 > > --- a/platform/linux-generic/Makefile
>                 > > +++ b/platform/linux-generic/Makefile
>                 > > @@ -32,12 +32,12 @@ ODP_ROOT = ../..
>                 > >  LIB_DIR  = ./lib
>                 > >  DOC_DIR  = ./doc
>                 > >
>                 > > -CFLAGS  += -I$(ODP_ROOT)/include
>                 > > -CFLAGS  += -I./include
>                 > > -CFLAGS  += -I./include/api
>                 > > +EXTRA_CFLAGS  += -I$(ODP_ROOT)/include
>                 > > +EXTRA_CFLAGS  += -I./include
>                 > > +EXTRA_CFLAGS  += -I./include/api
>                 > >
>                 > >  ifeq ($(ODP_HAVE_NETMAP),yes)
>                 > > -CFLAGS  += -DODP_HAVE_NETMAP
>                 > > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP
>                 > >  endif
>                 > >
>                 > >  include $(ODP_ROOT)/Makefile.inc
>                 > > @@ -92,7 +92,7 @@ $(DOC_DIR):
>                 > >  #
>                 > >  $(OBJ_DIR)/%.o: ./source/%.c
>                 > >          $(ECHO) Compiling $<
>                 > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>                 > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o
>         $@ $<
>                 > >
>                 > >  #
>                 > >  # Lib rule
>                 > > diff --git a/test/api_test/Makefile
>         b/test/api_test/Makefile
>                 > > index 15c18f4..741fbe4 100644
>                 > > --- a/test/api_test/Makefile
>                 > > +++ b/test/api_test/Makefile
>                 > > @@ -12,12 +12,12 @@ ODP_ATOMIC    = odp_atomic
>                 > >  ODP_SHM       = odp_shm
>                 > >  ODP_RING      = odp_ring
>                 > >
>                 > > -CFLAGS  +=
>         -I$(ODP_ROOT)/platform/linux-generic/include
>                 > > +EXTRA_CFLAGS  +=
>         -I$(ODP_ROOT)/platform/linux-generic/include
>                 > >
>                 > >  include ../Makefile.inc
>                 > >  include $(ODP_ROOT)/Makefile.inc
>                 > >
>                 > > -CFLAGS      += -I$(ODP_TEST_ROOT)/api_test
>                 > > +EXTRA_CFLAGS      += -I$(ODP_TEST_ROOT)/api_test
>                 > >
>                 > >  ATOMIC_OBJS  =
>                 > >  ATOMIC_OBJS += $(OBJ_DIR)/odp_common.o
>                 > > @@ -52,7 +52,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>                 > >  #
>                 > >  $(OBJ_DIR)/%.o: %.c
>                 > >          $(ECHO) Compiling $<
>                 > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>                 > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o
>         $@ $<
>                 > >
>                 > >  #
>                 > >  # Link rule
>                 > > diff --git a/test/example/Makefile
>         b/test/example/Makefile
>                 > > index d43e780..8064977 100644
>                 > > --- a/test/example/Makefile
>                 > > +++ b/test/example/Makefile
>                 > > @@ -30,7 +30,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>                 > >  #
>                 > >  $(OBJ_DIR)/%.o: %.c
>                 > >          $(ECHO) Compiling $<
>                 > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>                 > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o
>         $@ $<
>                 > >
>                 > >  #
>                 > >  # Link rule
>                 > > diff --git a/test/packet/Makefile
>         b/test/packet/Makefile
>                 > > index f1cb7d9..c66b37c 100644
>                 > > --- a/test/packet/Makefile
>                 > > +++ b/test/packet/Makefile
>                 > > @@ -9,7 +9,7 @@ ODP_APP  = odp_packet
>                 > >  include ../Makefile.inc
>                 > >  include $(ODP_ROOT)/Makefile.inc
>                 > >
>                 > > -CFLAGS  += -I$(ODP_TEST_ROOT)/packet
>                 > > +EXTRA_CFLAGS  += -I$(ODP_TEST_ROOT)/packet
>                 > >
>                 > >  OBJS     =
>                 > >  OBJS    += $(OBJ_DIR)/odp_example_pktio.o
>                 > > @@ -32,7 +32,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>                 > >  #
>                 > >  $(OBJ_DIR)/%.o: %.c
>                 > >          $(ECHO) Compiling $<
>                 > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>                 > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o
>         $@ $<
>                 > >
>                 > >  #
>                 > >  # Link rule
>                 > > diff --git a/test/packet_netmap/Makefile
>                 b/test/packet_netmap/Makefile
>                 > > index 5febb33..d92d113 100644
>                 > > --- a/test/packet_netmap/Makefile
>                 > > +++ b/test/packet_netmap/Makefile
>                 > > @@ -6,8 +6,8 @@
>                 > >  ODP_ROOT = ../..
>                 > >  ODP_APP  = odp_packet
>                 > >
>                 > > -CFLAGS  += -DODP_HAVE_NETMAP
>                 > > -CFLAGS  += -O0 -g
>                 > > +EXTRA_CFLAGS  += -DODP_HAVE_NETMAP
>                 > > +EXTRA_CFLAGS  += -O0 -g
>                 > >
>                 > >  include ../Makefile.inc
>                 > >  include $(ODP_ROOT)/Makefile.inc
>                 > > @@ -33,7 +33,7 @@ $(OBJ_DIR): $(DESTDIR)/lib/libodp.a
>                 > >  #
>                 > >  $(OBJ_DIR)/%.o: %.c
>                 > >          $(ECHO) Compiling $<
>                 > > -        $(CC) -c -MD $(CFLAGS) -o $@ $<
>                 > > +        $(CC) -c -MD $(EXTRA_CFLAGS) $(CFLAGS) -o
>         $@ $<
>                 > >
>                 > >  #
>                 > >  # Link rule
>                 > > --
>                 > > 1.8.3.2
>                 > >
>                 > >
>                 >
>                 > --
>                 > You received this message because you are subscribed
>         to the
>                 Google Groups "LNG ODP Sub-team - lng-odp@linaro.org
>         <mailto:lng-odp@linaro.org>
>                 <mailto:lng-odp@linaro.org
>         <mailto:lng-odp@linaro.org>>" group.
>
>                 > To unsubscribe from this group and stop receiving emails
>                 from it, send an email to
>         lng-odp+unsubscribe@linaro.org
>         <mailto:lng-odp%2Bunsubscribe@linaro.org>
>                 <mailto:lng-odp%2Bunsubscribe@linaro.org
>         <mailto:lng-odp%252Bunsubscribe@linaro.org>>.
>
>                 > To post to this group, send email to
>         lng-odp@linaro.org <mailto:lng-odp@linaro.org>
>                 <mailto:lng-odp@linaro.org <mailto:lng-odp@linaro.org>>.
>
>                 > Visit this group at
>         http://groups.google.com/a/linaro.org/group/lng-odp/.
>                 > To view this discussion on the web visit
>         https://groups.google.com/a/linaro.org/d/msgid/lng-odp/2b98325d-9086-4a5c-ada0-f5c7dfe81e17%40linaro.org.
>                 > For more options, visit
>         https://groups.google.com/a/linaro.org/groups/opt_out.
>
>                 Petri Mike,
>
>                 Would this solve the problems?
>
>                 For clang you need to specify: make CC=clang
>                 EXTRA_CFLAGS=-Wno-error
>                 would that work?
>
>                 Drawback with this solution is that we can't use -O3 +
>         ODP_DBG
>                 macro, do
>                 we want that? if yes, then we just need to change:
>                 ifeq ($(ODP_DEBUG), 1) to ifeq ($(a_good_name), 1)
>                 ideas of a good name?
>
>                 diff --git a/Makefile.inc b/Makefile.inc
>                 index 523385d..b35e030 100644
>                 --- a/Makefile.inc
>                 +++ b/Makefile.inc
>                 @@ -4,17 +4,24 @@
>                  # SPDX-License-Identifier:     BSD-3-Clause
>
>                  PLATFORM ?= linux-generic
>                 -CFLAGS  += -DODP_DEBUG=1
>                 -CFLAGS  += -O3
>                 -#CFLAGS  += -O0 -g
>                 +ODP_DEBUG ?= 0
>
>                  OBJ_DIR  = ./obj
>                  DESTDIR ?= $(ODP_ROOT)/build
>                 +EXTRA_CFLAGS ?=
>
>                  CFLAGS += -W -Wall -Werror -Wstrict-prototypes
>                 -Wmissing-prototypes
>                  CFLAGS += -Wmissing-declarations -Wold-style-definition
>                 -Wpointer-arith
>                  CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual
>                 -Wformat-nonliteral
>                  CFLAGS += -Wformat-security -Wundef -Wwrite-strings
>                 +CFLAGS += $(EXTRA_CFLAGS)
>                 +CFLAGS += -DODP_DEBUG=$(ODP_DEBUG)
>                 +
>                 +ifeq ($(ODP_DEBUG), 1)
>                 +  CFLAGS  += -O0 -g
>                 +else
>                 +  CFLAGS  += -O3
>                 +endif
>
>                  CC     ?= gcc
>                  LD     ?= gcc
>
>
>                 --
>                 Anders Roxell
>         anders.roxell@linaro.org <mailto:anders.roxell@linaro.org>
>         <mailto:anders.roxell@linaro.org
>         <mailto:anders.roxell@linaro.org>>
>                 M: +46 709 71 42 85 <tel:%2B46%20709%2071%2042%2085>
>         <tel:%2B46%20709%2071%2042%2085> | IRC: roxell
>
>
>                 --
>                 You received this message because you are subscribed
>         to the
>                 Google Groups "LNG ODP Sub-team - lng-odp@linaro.org
>         <mailto:lng-odp@linaro.org>
>                 <mailto:lng-odp@linaro.org
>         <mailto:lng-odp@linaro.org>>" group.
>
>                 To unsubscribe from this group and stop receiving
>         emails from
>                 it, send an email to lng-odp+unsubscribe@linaro.org
>         <mailto:lng-odp%2Bunsubscribe@linaro.org>
>                 <mailto:lng-odp%2Bunsubscribe@linaro.org
>         <mailto:lng-odp%252Bunsubscribe@linaro.org>>.
>
>                 To post to this group, send email to
>         lng-odp@linaro.org <mailto:lng-odp@linaro.org>
>                 <mailto:lng-odp@linaro.org <mailto:lng-odp@linaro.org>>.
>
>                 Visit this group at
>         http://groups.google.com/a/linaro.org/group/lng-odp/.
>                 To view this discussion on the web visit
>         https://groups.google.com/a/linaro.org/d/msgid/lng-odp/20140218112207.GD3517%408470w.
>                 For more options, visit
>         https://groups.google.com/a/linaro.org/groups/opt_out.
>
>
>
>         -- 
>         You received this message because you are subscribed to the
>         Google Groups "LNG ODP Sub-team - lng-odp@linaro.org
>         <mailto:lng-odp@linaro.org>" group.
>         To unsubscribe from this group and stop receiving emails from
>         it, send an email to lng-odp+unsubscribe@linaro.org
>         <mailto:lng-odp%2Bunsubscribe@linaro.org>.
>         To post to this group, send email to lng-odp@linaro.org
>         <mailto:lng-odp@linaro.org>.
>         Visit this group at
>         http://groups.google.com/a/linaro.org/group/lng-odp/.
>         To view this discussion on the web visit
>         https://groups.google.com/a/linaro.org/d/msgid/lng-odp/CADz3at18gL9DJ8kUKq4WNft0c4mHutEJ49Mj2ZXFULULpPA34w%40mail.gmail.com.
>
>
>         For more options, visit
>         https://groups.google.com/a/linaro.org/groups/opt_out.
>
>
>     -- 
>     You received this message because you are subscribed to the Google
>     Groups "LNG ODP Sub-team - lng-odp@linaro.org
>     <mailto:lng-odp@linaro.org>" group.
>     To unsubscribe from this group and stop receiving emails from it,
>     send an email to lng-odp+unsubscribe@linaro.org
>     <mailto:lng-odp%2Bunsubscribe@linaro.org>.
>     To post to this group, send email to lng-odp@linaro.org
>     <mailto:lng-odp@linaro.org>.
>     Visit this group at
>     http://groups.google.com/a/linaro.org/group/lng-odp/.
>     To view this discussion on the web visit
>     https://groups.google.com/a/linaro.org/d/msgid/lng-odp/5304B149.5060304%40linaro.org.
>
>
>     For more options, visit
>     https://groups.google.com/a/linaro.org/groups/opt_out.
>
>
diff mbox

Patch

diff --git a/Makefile.inc b/Makefile.inc
index 523385d..b35e030 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -4,17 +4,24 @@ 
 # SPDX-License-Identifier:     BSD-3-Clause
 
 PLATFORM ?= linux-generic
-CFLAGS  += -DODP_DEBUG=1
-CFLAGS  += -O3
-#CFLAGS  += -O0 -g
+ODP_DEBUG ?= 0
 
 OBJ_DIR  = ./obj
 DESTDIR ?= $(ODP_ROOT)/build
+EXTRA_CFLAGS ?=
 
 CFLAGS += -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes
 CFLAGS += -Wmissing-declarations -Wold-style-definition -Wpointer-arith
 CFLAGS += -Wcast-align -Wnested-externs -Wcast-qual -Wformat-nonliteral
 CFLAGS += -Wformat-security -Wundef -Wwrite-strings
+CFLAGS += $(EXTRA_CFLAGS)
+CFLAGS += -DODP_DEBUG=$(ODP_DEBUG)
+
+ifeq ($(ODP_DEBUG), 1)
+  CFLAGS  += -O0 -g
+else
+  CFLAGS  += -O3
+endif
 
 CC     ?= gcc
 LD     ?= gcc