GCC compilation broken - again

[plbossart]

commit 17c929e ('autotools: replace user variables with automake variables') adds options that GCC doesn't understand

gcc -DHAVE_CONFIG_H -I. -I../../src/include -fno-inline-functions -nostdlib -mlongcalls -mtext-section-literals -I /data/pbossart/SOF-public/sof-sdk/sof.git/src/arch/xtensa/include -I /data/pbossart/SOF-public/sof-sdk/sof.git/src/arch/xtensa/xtos -I /data/pbossart/SOF-public/sof-sdk/sof.git/src/platform/apollolake/include -I /data/pbossart/SOF-public/sof-sdk/sof.git/src/platform/intel/include -I /data/pbossart/SOF-public/sof-sdk/sof.git/src/platform/apollolake/include/arch -I /data/pbossart/SOF-public/sof-sdk/sof.git/src/include -I /data/pbossart/SOF-public/sof-sdk/sof.git/../xtensa-root/xtensa-apl-elf/include -O2 -g -Wall -Werror -Wl,-EL -Wmissing-prototypes -MT libinit_a-init.o -MD -MP -MF .deps/libinit_a-init.Tpo -c -o libinit_a-init.o test -f 'init.c' || echo './'init.c
gcc: error: unrecognized command line option ‘-mlongcalls’
gcc: error: unrecognized command line option ‘-mtext-section-literals’; did you mean ‘-fext-numeric-literals’?

[jajanusz]

It didn't work with GCC even before with flags from configure.ac (at least for me) or it was side effect, because imho we should check every flag that we add to CFLAGS etc with AC_COMPILE_IFELSE or AX_CHECK_COMPILE_FLAG then append it or not - to avoid issue that you get (invoking compiler with flags that it doesn't support) - it should had been done like this from the beginning if we want to support multiple compilers. Anyway I will make it work with mentioned macros - I just don't want to touch too many issues with one pull request, so I wanted to add it in next patches.

Can you also send me your config.log, so I can see how you invoke configure and what you get?
Thanks.

[jajanusz]

Also flags from top of configure.ac:

• -Wall -Werror -Wl,-EL -Wmissing-prototypes - every each of them should be checked if is supported and moved to AM_CFLAGS

• -O2 -g - do like above or leave it to be set by AC_PROG_CC

[lgirdwood]

All the -m flags are xtensa specific and should be added to AM_CFLAGS as part of the target selction logic in config.ac. The -W flags -O2 should be present on all builds.

[jajanusz]

The -W flags -O2 should be present on all builds.

Sorry, but I think it's wrong approach. Let me give you example.
Let's assume that I'm an user that cloned repo and I don't care about warnings, I wan't to just build this with my some custom flags, so I should be able to configure like:

./configure CFLAGS="-O0" ...

However with how it works now it will add always -Wall etc flags to CFLAGS, what we shouldn't force.
What's worse it will always add it at the end so it will look like "gcc ... -O0 -Wall -O2 ..." what makes it unable for user to override by order.

Maybe I'm missing some bigger picture and we should go against autotools conventions and force it.
Let me know what you think.

[lgirdwood]

We need to force CFLAGS to the correct flags for our FW, tools and host testbench. AC_PROG_CC is failing as it invoke the xensa compiler without the correct flags for building binary FW images (i.e. its using default CFLAGS for building userspace ELF).

[jajanusz]

If you only care about passing AC_PROG_CC, we can do something like:

SAVED_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS <OUR FLAGS>"
AC_PROG_CC
CFLAGS="$SAVED_CFLAGS"

Also compiler is not invoked at AC_PROG_CC statment, it's invoked first time when it's need, so atm it may be first invoked when autoconf checks if cmocka is available (cos it has to check if it can use function from linked lib).

We can have default CFLAGS that are like our current flags, but I'm not sure if we should force it they way it is now.

I can do as you want just want you to know that you make user unable to use one of feature of autoconf that is expected to work as I wrote (user should be able to override CFLAGS from configure step).

[plbossart]

@jajanusz: point taken, but in general people use ./configure CFLAGS="-O0" ... when they have access to a debugger. It's not really useful for firmware we mostly debug with traces, and if people have a need for this they can always edit the defaults. Let's just fix this and get out of this CI nightmare.

[keqiaozhang]

This issue is fixed.
sof-master:c4a9cca0a94969fbf1ecb42d660c0e40279d9ffa

[lgirdwood]

@keqiaozhang can you add all the build targets into CI, i.e. host, testbench and unit tests too. Thanks.

[jajanusz]

@keqiaozhang you need to build cmocka lib for every platform for UT, if you'll need help with this let me know

[keqiaozhang]

@jajanusz @lgirdwood I will follow up and keep you updated. Thanks~

[keqiaozhang]

This issue is fixed by #105

[cujomalainey]

I believe this issue has regressed again, I am experiencing this on master (at 597194f) in the docker container.

from ./scripts/xtensa-build-all.sh -l byt

gcc -DHAVE_CONFIG_H -I. -I../../src/include    -fno-inline-functions -nostdlib -mlongcalls -mtext-section-literals -I /usr/local/google/home/cujomalainey/sof/sof/src/arch/xtensa/include -I /usr/local/google/home/cujomalainey/sof/sof/src/arch/xtensa/xtos -I /usr/local/google/home/cujo
malainey/sof/sof/src/platform/baytrail/include   -I /usr/local/google/home/cujomalainey/sof/sof/src/platform/baytrail/include/arch -I /usr/local/google/home/cujomalainey/sof/sof/src/include -I /usr/local/google/home/cujomalainey/sof/sof/../xtensa-root/xtensa-byt-elf/include -O2 -g -W
all -Werror -Wl,-EL -Wmissing-prototypes -MT libinit_a-init.o -MD -MP -MF .deps/libinit_a-init.Tpo -c -o libinit_a-init.o `test -f 'init.c' || echo './'`init.c
gcc: error: unrecognized command line option ‘-mlongcalls’
gcc: error: unrecognized command line option ‘-mtext-section-literals’; did you mean ‘-fext-numeric-literals’?
Makefile:415: recipe for target 'libinit_a-init.o' failed

[cujomalainey]

Ignore the above comment, its something with docker-run script. If you jump into the containers bash then run the commands then things work fine.

[xiulipan]

@cujomalainey
Are you sure about using the docker-run script will result in above log?
It seems you are try to build without a cross-compiler exist.

gcc: error: unrecognized command line option ‘-mlongcalls’

The configure fallback to gcc when find no cross-compiler.

[cujomalainey]

@xiulipan it was an error on my behalf, somehow I omitted the container script after I just finished building in the container. Apologies for reviving a dead thread.