diff mbox

[U-Boot,v3] config.mk: fix -fstack-usage support test

Message ID 1381831886-26329-1-git-send-email-yamada.m@jp.panasonic.com
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Masahiro Yamada Oct. 15, 2013, 10:11 a.m. UTC
If -fstack-usage option is given for such architecures
that do not support it, gcc displays a warning message
but still exits with status 0.

This commits adds a new scripts to test -fstack-usage support
because we cannot rely on $(call cc-option,...) .

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
Cc: Tom Rini <trini@ti.com>
Cc: Michal Simek <monstr@monstr.eu>
Cc: Jeroen Hofstee <jeroen@myspectrum.nl>
Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
 config.mk                  |  5 +++--
 scripts/gcc-stack-usage.sh | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)
 create mode 100755 scripts/gcc-stack-usage.sh

Comments

Albert ARIBAUD Oct. 15, 2013, 10:21 a.m. UTC | #1
Hi Masahiro,

On Tue, 15 Oct 2013 19:11:26 +0900, Masahiro Yamada
<yamada.m@jp.panasonic.com> wrote:

> If -fstack-usage option is given for such architecures
> that do not support it, gcc displays a warning message
> but still exits with status 0.
> 
> This commits adds a new scripts to test -fstack-usage support
> because we cannot rely on $(call cc-option,...) .

Much better than the previous one (with a nitpick: superfluous "s" in
"This commits")... But now we lack the rationale part, which was
dropped rather than moved after the '---' line. 

Regarding the patch itself, why a shell script rather than a variant
of cc-option?

> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Tom Rini <trini@ti.com>
> Cc: Michal Simek <monstr@monstr.eu>
> Cc: Jeroen Hofstee <jeroen@myspectrum.nl>
> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> ---
>  config.mk                  |  5 +++--
>  scripts/gcc-stack-usage.sh | 18 ++++++++++++++++++
>  2 files changed, 21 insertions(+), 2 deletions(-)
>  create mode 100755 scripts/gcc-stack-usage.sh
> 
> diff --git a/config.mk b/config.mk
> index 3441387..8a82ab4 100644
> --- a/config.mk
> +++ b/config.mk
> @@ -279,8 +279,9 @@ CFLAGS_WARN := $(call cc-option,-Wno-format-nonliteral) \
>  CFLAGS += $(CFLAGS_WARN)
>  
>  # Report stack usage if supported
> -CFLAGS_STACK := $(call cc-option,-fstack-usage)
> -CFLAGS += $(CFLAGS_STACK)
> +ifeq ($(shell $(SHELL) $(SRCTREE)/scripts/gcc-stack-usage.sh $(CC)), y)
> +	CFLAGS += -fstack-usage
> +endif
>  
>  BCURDIR = $(subst $(SRCTREE)/,,$(CURDIR:$(obj)%=%))
>  
> diff --git a/scripts/gcc-stack-usage.sh b/scripts/gcc-stack-usage.sh
> new file mode 100755
> index 0000000..53eb10a
> --- /dev/null
> +++ b/scripts/gcc-stack-usage.sh
> @@ -0,0 +1,18 @@
> +#!/bin/sh
> +# Test for gcc '-fstack-usage' support
> +# Copyright (C) 2013, Masahiro Yamada <yamada.m@jp.panasonic.com>
> +#
> +# SPDX-License-Identifier:	GPL-2.0+
> +#
> +
> +TMP=${OBJTREE}/"$$"
> +
> +cat << "END" | $@ -Werror -fstack-usage -x c - -c -o $TMP >/dev/null 2>&1 \
> +							&& echo "y"
> +int main(void)
> +{
> +	return 0;
> +}
> +END
> +
> +rm -f $TMP $TMP.su


Amicalement,
Masahiro Yamada Oct. 15, 2013, 11:15 a.m. UTC | #2
Hello Albert


> Much better than the previous one (with a nitpick: superfluous "s" in
> "This commits")... But now we lack the rationale part, which was
> dropped rather than moved after the '---' line. 

I will add the rationale part in v4 along with s/commits/commit/.
Thanks.


> Regarding the patch itself, why a shell script rather than a variant
> of cc-option?

We cannot simply test -fstack-option support by $(call cc-option,...)

This was mentioned in version 1 discussion,
(I should have written this below the '---')

> 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


I chose a shell script because I wanted to keep Makefile cleaner
and this is the way Linux Kernel often uses.

Please refer to
 - scripts/gcc-goto.sh
 - scripts/gcc-version.sh
etc. of Linux Kernel.


Best Regards
Masahiro Yamada
Albert ARIBAUD Oct. 15, 2013, 11:29 a.m. UTC | #3
Hi Masahiro,

> > Regarding the patch itself, why a shell script rather than a variant
> > of cc-option?
> 
> I chose a shell script because I wanted to keep Makefile cleaner
> and this is the way Linux Kernel often uses.
> 
> Please refer to
>  - scripts/gcc-goto.sh
>  - scripts/gcc-version.sh
> etc. of Linux Kernel.

Understood, thanks.

> Best Regards
> Masahiro Yamada

Amicalement,
diff mbox

Patch

diff --git a/config.mk b/config.mk
index 3441387..8a82ab4 100644
--- a/config.mk
+++ b/config.mk
@@ -279,8 +279,9 @@  CFLAGS_WARN := $(call cc-option,-Wno-format-nonliteral) \
 CFLAGS += $(CFLAGS_WARN)
 
 # Report stack usage if supported
-CFLAGS_STACK := $(call cc-option,-fstack-usage)
-CFLAGS += $(CFLAGS_STACK)
+ifeq ($(shell $(SHELL) $(SRCTREE)/scripts/gcc-stack-usage.sh $(CC)), y)
+	CFLAGS += -fstack-usage
+endif
 
 BCURDIR = $(subst $(SRCTREE)/,,$(CURDIR:$(obj)%=%))
 
diff --git a/scripts/gcc-stack-usage.sh b/scripts/gcc-stack-usage.sh
new file mode 100755
index 0000000..53eb10a
--- /dev/null
+++ b/scripts/gcc-stack-usage.sh
@@ -0,0 +1,18 @@ 
+#!/bin/sh
+# Test for gcc '-fstack-usage' support
+# Copyright (C) 2013, Masahiro Yamada <yamada.m@jp.panasonic.com>
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+TMP=${OBJTREE}/"$$"
+
+cat << "END" | $@ -Werror -fstack-usage -x c - -c -o $TMP >/dev/null 2>&1 \
+							&& echo "y"
+int main(void)
+{
+	return 0;
+}
+END
+
+rm -f $TMP $TMP.su