[U-Boot] [PATCH] config.mk: enable -fstack-usage only when it is desired

[U-Boot] [PATCH] config.mk: enable -fstack-usage only when it is desired

[Masahiro Yamada]

As far as I know, gcc does not support -fstack-usage
for some targets such as blackfin, m68k, microblaze, etc.

If -fstack-usage option is given for those targets,
gcc displays a warning message as follows:

    warning: -fstack-usage not supported for this target [enabled by default]

But it still exits with status 0.

So,

    # Report stack usage if supported
    CFLAGS_STACK := $(call cc-option,-fstack-usage)
    CFLAGS += $(CFLAGS_STACK)

does not work as we expect because cc-option sees exit status
to judge whether the given option is supported or not.

To suppress warnings for such targets that -fstack-usage is not supported,
this commit surrounds the concerned lines with
ifdef CONFIG_CC_STACKUSAGE .. endif.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
Cc: Tom Rini <trini@ti.com>
---
 config.mk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/config.mk b/config.mk
index 48913f6..d405ab4 100644
--- a/config.mk
+++ b/config.mk
@@ -278,9 +278,9 @@ CFLAGS_WARN := $(call cc-option,-Wno-format-nonliteral) \
 	       $(call cc-option,-Wno-format-security)
 CFLAGS += $(CFLAGS_WARN)
 
-# Report stack usage if supported
-CFLAGS_STACK := $(call cc-option,-fstack-usage)
-CFLAGS += $(CFLAGS_STACK)
+ifdef CONFIG_CC_STACKUSAGE
+CFLAGS += $(call cc-option,-fstack-usage)
+endif
 
 BCURDIR = $(subst $(SRCTREE)/,,$(CURDIR:$(obj)%=%))
 
-- 
1.8.1.2

[U-Boot] [PATCH] config.mk: enable -fstack-usage only when it is desired

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/25/2013 10:17 PM, Masahiro Yamada wrote:
> As far as I know, gcc does not support -fstack-usage
> for some targets such as blackfin, m68k, microblaze, etc.
> 
> If -fstack-usage option is given for those targets,
> gcc displays a warning message as follows:
> 
>     warning: -fstack-usage not supported for this target [enabled by default]
> 
> But it still exits with status 0.
> 
> So,
> 
>     # Report stack usage if supported
>     CFLAGS_STACK := $(call cc-option,-fstack-usage)
>     CFLAGS += $(CFLAGS_STACK)
> 
> does not work as we expect because cc-option sees exit status
> to judge whether the given option is supported or not.
> 
> To suppress warnings for such targets that -fstack-usage is not supported,
> this commit surrounds the concerned lines with
> ifdef CONFIG_CC_STACKUSAGE .. endif.
> 
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Tom Rini <trini@ti.com>

The problem here is that except for whatever arches don't support
- -fstack-usage, we always want to generate this information, to assist in
debugging stack usage problems (which can be a real problem within SPL
for example).  How about ifneq($(CONFIG_M68K)$(CONFIG_...),y) ... endif
around it?

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSRCAOAAoJENk4IS6UOR1WQ3AP/2kWJVnQqpMXUvX5XJ/jXzsW
tNx0Rj+JZcXgH8Nl4EAPj/oBQEA6H5Qnic0pEE9G8HpkrgEsCfqn9wAsbvN/8pCw
ZaU7zkORh4ieAz1w+ZJeJgx8rksxWNbVqFzttwDrOtvOK2s1WMLi7zfypw7DPoc/
ZPuvZJRHbbEFMqf9Y6Mo2smnM9uo+z+JnsNBio2Fau5A8VoZIaAI8Z2rob352oQ7
1MM2J1ncrmxEOO178CEbLD0dAJWqNZFNfpSZpfb/V+vYMaAMmCgYALXuBbue+JDy
PfeRxPD8iXiv5G40dQce0mjoUVDujnuOAlralaQBv0okegnGgF6ghf+bWt+sxTuT
h9/nY3i0ToRplsk2GKNAMVDNzonKBgk9BAz78ZVc6qMaLi6g9d8XVfXhoLLbWLPC
XEEZOYlflt6pnsybQjhux0UOm6hyy1moqll6GukEVmUWtYrMjimPLy2uKmeTdPbU
vnHOCcRHUD8SPBAyA9voOlwDJteusVH6FAsNoXLSSjnLI+vrBLhXTlUH/kRXMuA3
JRSLLBcwRi/VDxe0gWjSGbmTnOiWVKkg9nLNBdqMnqJI8vTYZjwDxJ50RLUQJWSq
N+kB3yZ/+ElhPi7vjJc8hy+sHUNfoVN1z6TyfY1fRxJfXXccDyBUE8eqEOWxBGg7
YldF9zwHiqjs2m/WuiIY
=YRCk
-----END PGP SIGNATURE-----

[U-Boot] [PATCH] config.mk: enable -fstack-usage only when it is desired

[Michal Simek]

On 09/26/2013 01:52 PM, Tom Rini wrote:
> On 09/25/2013 10:17 PM, Masahiro Yamada wrote:
>> As far as I know, gcc does not support -fstack-usage
>> for some targets such as blackfin, m68k, microblaze, etc.

Please remove microblaze from this list.
Our gcc support fstack-usage.

If your microblaze gcc doesn't support this flag
then upgrade gcc version.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130926/afa49300/attachment.pgp>

[U-Boot] [PATCH] config.mk: enable -fstack-usage only when it is desired

[Masahiro Yamada]

Hello Michal

> Please remove microblaze from this list.
> Our gcc support fstack-usage.

I will. Thanks for helpful information!


> If your microblaze gcc doesn't support this flag
> then upgrade gcc version.

I tested microblaze-linux-gcc 4.8.0
I downloaded from ftp://ftp.kernel.org/pub/tools/crosstool/index.html

Can I get the prebuilt newer version at some download sites
or need to build from source code?


Best Regards
Masahiro Yamada

[U-Boot] [PATCH] config.mk: enable -fstack-usage only when it is desired

[Michal Simek]

Hi,

David: Can you please tell to Masahiro where he can download
toolchain in any format which will support -fstack-usage.

I have microblazeel-xilinx-linux-gnu-gcc (crosstool-NG 1.18.0) 4.6.4 20120924 (prerelease)

Thanks,
Michal

On 09/27/2013 04:23 AM, Masahiro Yamada wrote:
> Hello Michal
> 
>> Please remove microblaze from this list.
>> Our gcc support fstack-usage.
> 
> I will. Thanks for helpful information!
> 
> 
>> If your microblaze gcc doesn't support this flag
>> then upgrade gcc version.
> 
> I tested microblaze-linux-gcc 4.8.0
> I downloaded from ftp://ftp.kernel.org/pub/tools/crosstool/index.html
> 
> Can I get the prebuilt newer version at some download sites
> or need to build from source code?
> 
> 
> Best Regards
> Masahiro Yamada
> 


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130927/8a6c6cda/attachment.pgp>

[U-Boot] [PATCH] config.mk: enable -fstack-usage only when it is desired

[Masahiro Yamada]

Hello Tom.

> The problem here is that except for whatever arches don't support
> - -fstack-usage, we always want to generate this information, to assist in
> debugging stack usage problems (which can be a real problem within SPL
> for example).  How about ifneq($(CONFIG_M68K)$(CONFIG_...),y) ... endif
> around it?

OK. This works.

But I'm kind of afraid ifneq($(CONFIG_M68K)$(CONFIG_...),y) ... endif
might be too arch-specific to be written in the top config.mk.

Instead, we can add
CONFIG_CC_STACKUSAGE=y
to arch/{arm, powerpc, x86, microblaze ...}/config.mk



Best Regards
Masahiro Yamada

[U-Boot] [PATCH] config.mk: enable -fstack-usage only when it is desired

[Jeroen Hofstee]

Hello Masahiro,

On 09/27/2013 04:09 AM, Masahiro Yamada wrote:
>
>> The problem here is that except for whatever arches don't support
>> - -fstack-usage, we always want to generate this information, to assist in
>> debugging stack usage problems (which can be a real problem within SPL
>> for example).  How about ifneq($(CONFIG_M68K)$(CONFIG_...),y) ... endif
>> around it?
> OK. This works.
>
> But I'm kind of afraid ifneq($(CONFIG_M68K)$(CONFIG_...),y) ... endif
> might be too arch-specific to be written in the top config.mk.
>
> Instead, we can add
> CONFIG_CC_STACKUSAGE=y
> to arch/{arm, powerpc, x86, microblaze ...}/config.mk

I guess below patch should work as well. Anyone aware
of any side affects it might cause?

---
Regards,
Jeroen

diff --git a/config.mk b/config.mk
index 48913f6..177f685 100644
--- a/config.mk
+++ b/config.mk
@@ -117,7 +117,7 @@ CC_TEST_OFILE := 
$(OBJTREE)/include/generated/cc_test_file.o
  -include $(CC_OPTIONS_CACHE_FILE)

  cc-option-sys = $(shell mkdir -p $(dir $(CC_TEST_OFILE)); \
-               if $(CC) $(CFLAGS) $(1) -S -xc /dev/null -o 
$(CC_TEST_OFILE) \
+               if $(CC) -Werror $(CFLAGS) $(1) -S -xc /dev/null -o 
$(CC_TEST_OFILE) \
                 > /dev/null 2>&1; then \
                 echo 'CC_OPTIONS += $(strip $1)' >> 
$(CC_OPTIONS_CACHE_FILE); \
                 echo "$(1)"; fi)

[U-Boot] [PATCH] config.mk: enable -fstack-usage only when it is desired

[Masahiro Yamada]

Hello Jeroen,

> diff --git a/config.mk b/config.mk
> index 48913f6..177f685 100644
> --- a/config.mk
> +++ b/config.mk
> @@ -117,7 +117,7 @@ CC_TEST_OFILE := 
> $(OBJTREE)/include/generated/cc_test_file.o
>   -include $(CC_OPTIONS_CACHE_FILE)
> 
>   cc-option-sys = $(shell mkdir -p $(dir $(CC_TEST_OFILE)); \
> -               if $(CC) $(CFLAGS) $(1) -S -xc /dev/null -o 
> $(CC_TEST_OFILE) \
> +               if $(CC) -Werror $(CFLAGS) $(1) -S -xc /dev/null -o 
> $(CC_TEST_OFILE) \
>                  > /dev/null 2>&1; then \
>                  echo 'CC_OPTIONS += $(strip $1)' >> 
> $(CC_OPTIONS_CACHE_FILE); \
>                  echo "$(1)"; fi)



It looks like this was already suggested by Tom too.
See http://patchwork.ozlabs.org/patch/183174/

I tested this patch but unfortunatelly it did not work. 


I downloaded bfin-linux-gcc 4.6.3 from
ftp://ftp.kernel.org/pub/tools/crosstool/index.html

I added -Werror to config.mk and I tried:

    CROSS_COMPILE=bfin-linux- ./MAKEALL -a blackfin

The log message was still sprinkled with lots of warnings like
warning: -fstack-usage not supported for this target [enabled by default]

So, I looked into it more closely and
I found gcc can compile the input file /dev/null successfully
even if -fstack-usage is not supported.


    $ bfin-uclinux-gcc -fstack-usage -S -xc /dev/null 
    $ echo $?
    0
    $ bfin-uclinux-gcc -Werror -fstack-usage -S -xc /dev/null 
    $ echo $?
    0



Instead of /dev/null, I prepared a very simple C source code as an input.

    $ cat test.c
    int test(void)
    {
        return 0;
    }
    $ bfin-uclinux-gcc -fstack-usage -S -xc test.c
    test.c: In function 'test':
    test.c:4:1: warning: -fstack-usage not supported for this target [enabled by default]
    $ echo $?
    0
    $ bfin-uclinux-gcc -fstack-usage -Werror -S -xc test.c
    test.c: In function 'test':
    test.c:4:1: error: -fstack-usage not supported for this target [-Werror]
    cc1: all warnings being treated as errors
    $ echo $?
    1

This time we can detect unsupported -fstack-usage by the exit status.

So, along with -Werror, I think /dev/null should also be replaced
with some _REAL_ C source file.
But my concern about this treak is we might go far from Kbuild.
In Linux Kernel, cc-option is impilemented in scripts/Kbuild.include.
It uses /dev/null as input.
I wish U-Boot would someday adopt (not nessarily the same but very
similar) Kbuild.

If you see Linux Kernel top Makefile,
many of optional CFLAGS are provided thru cc-option and
also can be turned on/off by CONFIG_ switch.

That is one reason I suggested to introduce CONFIG_CC_STACKUSAGE option.
But this is just my opinion and I would not necessarily stick to this idea.
Please feel free to suggest your thought.


Best Regards
Masahiro Yamada

[U-Boot] [PATCH] config.mk: enable -fstack-usage only when it is desired

[Jeroen Hofstee]

Hello Masahiro,

On 09/30/2013 10:01 AM, Masahiro Yamada wrote:
> Hello Jeroen,
>
>> diff --git a/config.mk b/config.mk
>> index 48913f6..177f685 100644
>> --- a/config.mk
>> +++ b/config.mk
>> @@ -117,7 +117,7 @@ CC_TEST_OFILE :=
>> $(OBJTREE)/include/generated/cc_test_file.o
>>    -include $(CC_OPTIONS_CACHE_FILE)
>>
>>    cc-option-sys = $(shell mkdir -p $(dir $(CC_TEST_OFILE)); \
>> -               if $(CC) $(CFLAGS) $(1) -S -xc /dev/null -o
>> $(CC_TEST_OFILE) \
>> +               if $(CC) -Werror $(CFLAGS) $(1) -S -xc /dev/null -o
>> $(CC_TEST_OFILE) \
>>                   > /dev/null 2>&1; then \
>>                   echo 'CC_OPTIONS += $(strip $1)' >>
>> $(CC_OPTIONS_CACHE_FILE); \
>>                   echo "$(1)"; fi)
>
>
> It looks like this was already suggested by Tom too.
> See http://patchwork.ozlabs.org/patch/183174/
>
> I tested this patch but unfortunatelly it did not work.
>
>
> I downloaded bfin-linux-gcc 4.6.3 from
> ftp://ftp.kernel.org/pub/tools/crosstool/index.html
>
> I added -Werror to config.mk and I tried:
>
>      CROSS_COMPILE=bfin-linux- ./MAKEALL -a blackfin
>
> The log message was still sprinkled with lots of warnings like
> warning: -fstack-usage not supported for this target [enabled by default]
>
> So, I looked into it more closely and
> I found gcc can compile the input file /dev/null successfully
> even if -fstack-usage is not supported.
>
>
>      $ bfin-uclinux-gcc -fstack-usage -S -xc /dev/null
>      $ echo $?
>      0
>      $ bfin-uclinux-gcc -Werror -fstack-usage -S -xc /dev/null
>      $ echo $?
>      0
>
>
>
> Instead of /dev/null, I prepared a very simple C source code as an input.
>
>      $ cat test.c
>      int test(void)
>      {
>          return 0;
>      }
>      $ bfin-uclinux-gcc -fstack-usage -S -xc test.c
>      test.c: In function 'test':
>      test.c:4:1: warning: -fstack-usage not supported for this target [enabled by default]
>      $ echo $?
>      0
>      $ bfin-uclinux-gcc -fstack-usage -Werror -S -xc test.c
>      test.c: In function 'test':
>      test.c:4:1: error: -fstack-usage not supported for this target [-Werror]
>      cc1: all warnings being treated as errors
>      $ echo $?
>      1
>
> This time we can detect unsupported -fstack-usage by the exit status.
>
> So, along with -Werror, I think /dev/null should also be replaced
> with some _REAL_ C source file.
> But my concern about this treak is we might go far from Kbuild.
> In Linux Kernel, cc-option is impilemented in scripts/Kbuild.include.
> It uses /dev/null as input.
> I wish U-Boot would someday adopt (not nessarily the same but very
> similar) Kbuild.
>
> If you see Linux Kernel top Makefile,
> many of optional CFLAGS are provided thru cc-option and
> also can be turned on/off by CONFIG_ switch.
>
> That is one reason I suggested to introduce CONFIG_CC_STACKUSAGE option.
> But this is just my opinion and I would not necessarily stick to this idea.
> Please feel free to suggest your thought.
>
>

I tried to google this a bit, one thing I found was
http://sourceware.org/bugzilla/show_bug.cgi?id=5210, but I fail
to (easily) find a reliable gcc reference. Perhaps we should
file a bug? -Werror doesn't work with /dev/null when
-fstack-usage is not supported...

I cannot comment on your concern regarding Kbuild, since I
am unaware of its internals. I just suggested above since
it would be a cleaner approach (if it worked...).

Regards,
Jeroen

[U-Boot] [PATCH] config.mk: enable -fstack-usage only when it is desired

[Masahiro Yamada]

Hello Jeroen

> 
> I tried to google this a bit, one thing I found was
> http://sourceware.org/bugzilla/show_bug.cgi?id=5210, but I fail
> to (easily) find a reliable gcc reference. Perhaps we should
> file a bug? -Werror doesn't work with /dev/null when
> -fstack-usage is not supported...

I might be a bug but I cannot say with confident.
Anyway, we need to do something
with the current gcc behavior.


> I cannot comment on your concern regarding Kbuild, since I
> am unaware of its internals. I just suggested above since
> it would be a cleaner approach (if it worked...).

I posted
version2 : config.mk: disable -fstack-usage for blackfin and m68k
and
version3 : config.mk: fix -fstack-usage support test


I think the latter is close to your idea and
I believe it's better.


Best Regards
Masahiro Yamada