> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, October 26, 2016 9:54 AM
> 
> 2016-10-18 21:11, Rasesh Mody:
> > Fix 32 bit compilation for gcc version 4.3.4.
> >
> > Fixes: ec94dbc57362 ("qede: add base driver")
> >
> > Signed-off-by: Rasesh Mody <rasesh.mody at qlogic.com>
> [...]
> >  ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
> > +ifeq ($(shell gcc -Wno-unused-but-set-variable -Werror -E - <
> > +/dev/null > /dev/null 2>&1; echo $$?),0)
> >  CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable
> > +endif
> >  CFLAGS_BASE_DRIVER += -Wno-missing-declarations
> > +ifeq ($(shell gcc -Wno-maybe-uninitialized -Werror -E - < /dev/null >
> > +/dev/null 2>&1; echo $$?),0)
> >  CFLAGS_BASE_DRIVER += -Wno-maybe-uninitialized
> > +endif
> >  CFLAGS_BASE_DRIVER += -Wno-strict-prototypes  ifeq ($(shell test
> > $(GCC_VERSION) -ge 60 && echo 1), 1)  CFLAGS_BASE_DRIVER +=
> > -Wno-shift-negative-value
> 
> What the hell are you doing here?

In one of our compilation testing on i586, we have gcc version 4.3.4. This 
version of gcc gives us following errors:

cc1: error: unrecognized command line option "-Wno-unused-but-set-variable"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"

-Wno-unused-but-set-variable option was added only in gcc version 5.1.0
-Wno-maybe-uninitialized option was added only in gcc version 4.7.0

All that above change does is that it checks if -Wno-unused-but-set-variable 
and -Wno-maybe-uninitialized options are available with gcc only then include 
them for compilation.

> 1/ You should better fix "unused-but-set-variable" errors 2/ It won't work
> when cross-compiling because you do not use $(CC)
>       in $(shell gcc

We tested on gcc version 6.2.0 on x86_64 without applying this patch. Errors 
related to "unused-but-set-variable" option were not seen. The only errors we 
saw are as noted above due to an older version of gcc.
We do use $(shell gcc, however, it is used under ifeq 
($(CONFIG_RTE_TOOLCHAIN_GCC),y), so, I believe it should work when 
cross-compiling. For example, in one of our compilation testing on clang 
version 3.8.0, with this patch applied, we did not see any errors. Please let 
us know if you see otherwise.

However, I do agree it is better to use $(CC). We could change that with a 
follow on patch.

Thanks!
-Rasesh

> 
> I really do not want to look at the qede patches.
> But each time my eyes stop on one of them, I'm struggling.

Reply via email to