Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1635429 > unrolled thread

[PATCH] x86/build: don't add -maccumulate-outgoing-args w/o compiler support

Started byNick Desaulniers <nick.desaulniers@gmail.com>
First post2017-05-04 08:30 +0200
Last post2017-05-09 08:50 +0200
Articles 11 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] x86/build: don't add -maccumulate-outgoing-args w/o compiler support Nick Desaulniers <nick.desaulniers@gmail.com> - 2017-05-04 08:30 +0200
    Re: [PATCH] x86/build: don't add -maccumulate-outgoing-args w/o  compiler support Josh Poimboeuf <jpoimboe@redhat.com> - 2017-05-04 21:20 +0200
      [Patch v2] x86/build: require only gcc use -maccumulate-outgoing-args Nick Desaulniers <nick.desaulniers@gmail.com> - 2017-05-05 05:20 +0200
        Re: [Patch v2] x86/build: require only gcc use  -maccumulate-outgoing-args Ingo Molnar <mingo@kernel.org> - 2017-05-05 08:30 +0200
          Re: [Patch v2] x86/build: require only gcc use -maccumulate-outgoing-args hpa@zytor.com - 2017-05-05 08:50 +0200
            Re: [Patch v2] x86/build: require only gcc use  -maccumulate-outgoing-args Josh Poimboeuf <jpoimboe@redhat.com> - 2017-05-05 15:10 +0200
              [Patch v3] x86/build: don't add -maccumulate-outgoing-args w/o compiler support Nick Desaulniers <nick.desaulniers@gmail.com> - 2017-05-06 05:10 +0200
                Re: [Patch v3] x86/build: don't add -maccumulate-outgoing-args w/o  compiler support Josh Poimboeuf <jpoimboe@redhat.com> - 2017-05-08 05:30 +0200
                  [Patch v4] x86/build: don't add -maccumulate-outgoing-args w/o compiler support Nick Desaulniers <nick.desaulniers@gmail.com> - 2017-05-09 05:40 +0200
                    Re: [Patch v4] x86/build: don't add -maccumulate-outgoing-args w/o  compiler support Josh Poimboeuf <jpoimboe@redhat.com> - 2017-05-09 05:50 +0200
                    [tip:x86/urgent] x86/build: Don't add -maccumulate-outgoing-args  w/o compiler support tip-bot for Nick Desaulniers <tipbot@zytor.com> - 2017-05-09 08:50 +0200

#1635429 — [PATCH] x86/build: don't add -maccumulate-outgoing-args w/o compiler support

FromNick Desaulniers <nick.desaulniers@gmail.com>
Date2017-05-04 08:30 +0200
Subject[PATCH] x86/build: don't add -maccumulate-outgoing-args w/o compiler support
Message-ID<tDiqZ-8aF-5@gated-at.bofh.it>
Otherwise other compilers, like Clang, are prevented from compiling the
kernel. This flag was introduced in
3f135e57a4f76d24ae8d8a490314331f0ced40c5.

Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
---
 arch/x86/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 4430dd4..5a0ac84 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -179,7 +179,7 @@ ifdef CONFIG_JUMP_LABEL
 endif
 
 ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
-	KBUILD_CFLAGS += -maccumulate-outgoing-args
+	KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args,)
 endif
 
 # Stackpointer is addressed different for 32 bit and 64 bit x86
-- 
2.9.3

[toc] | [next] | [standalone]

#1635983 — Re: [PATCH] x86/build: don't add -maccumulate-outgoing-args w/o compiler support

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-05-04 21:20 +0200
SubjectRe: [PATCH] x86/build: don't add -maccumulate-outgoing-args w/o compiler support
Message-ID<tDus9-7Mu-15@gated-at.bofh.it>
In reply to#1635429
On Wed, May 03, 2017 at 11:23:41PM -0700, Nick Desaulniers wrote:
> Otherwise other compilers, like Clang, are prevented from compiling the
> kernel. This flag was introduced in
> 3f135e57a4f76d24ae8d8a490314331f0ced40c5.
> 
> Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
> ---
>  arch/x86/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 4430dd4..5a0ac84 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -179,7 +179,7 @@ ifdef CONFIG_JUMP_LABEL
>  endif
>  
>  ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
> -	KBUILD_CFLAGS += -maccumulate-outgoing-args
> +	KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args,)
>  endif

I think this might silently cover up errors for older versions of gcc.
For example, what happens if '-maccumulate-outgoing-args' is needed, but
the version of gcc doesn't support it.  In that case I'd rather see gcc
report an error that the flag isn't supported, instead of it getting
silently disabled.

So how about this instead?

	KBUILD_CFLAGS += $(if $(filter gcc,$(cc-name)),-maccumulate-outgoing-args)

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1636139 — [Patch v2] x86/build: require only gcc use -maccumulate-outgoing-args

FromNick Desaulniers <nick.desaulniers@gmail.com>
Date2017-05-05 05:20 +0200
Subject[Patch v2] x86/build: require only gcc use -maccumulate-outgoing-args
Message-ID<tDBWG-4lL-5@gated-at.bofh.it>
In reply to#1635983
Other compilers, like clang, treat unknown compiler flags as errors.

Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
---
 arch/x86/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 4430dd489620..12757a252e6b 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -179,7 +179,7 @@ ifdef CONFIG_JUMP_LABEL
 endif
 
 ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
-	KBUILD_CFLAGS += -maccumulate-outgoing-args
+	KBUILD_CFLAGS += $(if $(filter gcc,$(cc-name)),-maccumulate-outgoing-args)
 endif
 
 # Stackpointer is addressed different for 32 bit and 64 bit x86
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1636187 — Re: [Patch v2] x86/build: require only gcc use -maccumulate-outgoing-args

FromIngo Molnar <mingo@kernel.org>
Date2017-05-05 08:30 +0200
SubjectRe: [Patch v2] x86/build: require only gcc use -maccumulate-outgoing-args
Message-ID<tDEUy-6lR-3@gated-at.bofh.it>
In reply to#1636139
* Nick Desaulniers <nick.desaulniers@gmail.com> wrote:

> Other compilers, like clang, treat unknown compiler flags as errors.
> 
> Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
> ---
>  arch/x86/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 4430dd489620..12757a252e6b 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -179,7 +179,7 @@ ifdef CONFIG_JUMP_LABEL
>  endif
>  
>  ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
> -	KBUILD_CFLAGS += -maccumulate-outgoing-args
> +	KBUILD_CFLAGS += $(if $(filter gcc,$(cc-name)),-maccumulate-outgoing-args)
>  endif
>  
>  # Stackpointer is addressed different for 32 bit and 64 bit x86

The justification Josh gave for this pattern should be put into a comment and into 
the changelog as well.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1636194 — Re: [Patch v2] x86/build: require only gcc use -maccumulate-outgoing-args

Fromhpa@zytor.com
Date2017-05-05 08:50 +0200
SubjectRe: [Patch v2] x86/build: require only gcc use -maccumulate-outgoing-args
Message-ID<tDFdU-6v3-1@gated-at.bofh.it>
In reply to#1636187
On May 4, 2017 11:23:33 PM PDT, Ingo Molnar <mingo@kernel.org> wrote:
>
>* Nick Desaulniers <nick.desaulniers@gmail.com> wrote:
>
>> Other compilers, like clang, treat unknown compiler flags as errors.
>> 
>> Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
>> ---
>>  arch/x86/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>> index 4430dd489620..12757a252e6b 100644
>> --- a/arch/x86/Makefile
>> +++ b/arch/x86/Makefile
>> @@ -179,7 +179,7 @@ ifdef CONFIG_JUMP_LABEL
>>  endif
>>  
>>  ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
>> -	KBUILD_CFLAGS += -maccumulate-outgoing-args
>> +	KBUILD_CFLAGS += $(if $(filter
>gcc,$(cc-name)),-maccumulate-outgoing-args)
>>  endif
>>  
>>  # Stackpointer is addressed different for 32 bit and 64 bit x86
>
>The justification Josh gave for this pattern should be put into a
>comment and into 
>the changelog as well.
>
>Thanks,
>
>	Ingo

However, I don't think Josh's explanation is correct.  I am pretty sure it is a performance issue, not a correctness issue, and besides, a version of gcc that old won't be able to compile the kernel for other reasons, as evidenced by the fact that noone has complained about this option being mandatory.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[toc] | [prev] | [next] | [standalone]

#1636366 — Re: [Patch v2] x86/build: require only gcc use -maccumulate-outgoing-args

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-05-05 15:10 +0200
SubjectRe: [Patch v2] x86/build: require only gcc use -maccumulate-outgoing-args
Message-ID<tDL9D-28E-7@gated-at.bofh.it>
In reply to#1636194
On Thu, May 04, 2017 at 11:38:57PM -0700, hpa@zytor.com wrote:
> On May 4, 2017 11:23:33 PM PDT, Ingo Molnar <mingo@kernel.org> wrote:
> >
> >* Nick Desaulniers <nick.desaulniers@gmail.com> wrote:
> >
> >> Other compilers, like clang, treat unknown compiler flags as errors.
> >> 
> >> Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
> >> ---
> >>  arch/x86/Makefile | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> >> index 4430dd489620..12757a252e6b 100644
> >> --- a/arch/x86/Makefile
> >> +++ b/arch/x86/Makefile
> >> @@ -179,7 +179,7 @@ ifdef CONFIG_JUMP_LABEL
> >>  endif
> >>  
> >>  ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
> >> -	KBUILD_CFLAGS += -maccumulate-outgoing-args
> >> +	KBUILD_CFLAGS += $(if $(filter
> >gcc,$(cc-name)),-maccumulate-outgoing-args)
> >>  endif
> >>  
> >>  # Stackpointer is addressed different for 32 bit and 64 bit x86
> >
> >The justification Josh gave for this pattern should be put into a
> >comment and into 
> >the changelog as well.
> >
> >Thanks,
> >
> >	Ingo
> 
> However, I don't think Josh's explanation is correct.  I am pretty
> sure it is a performance issue, not a correctness issue

Why wouldn't it be a correctness issue?  The option is needed in a few
cases (involving older versions of gcc and certain configs) to avoid
some bugs (see the Makefile for more details).

> and besides, a version of gcc that old won't be able to compile the
> kernel for other reasons, as evidenced by the fact that noone has
> complained about this option being mandatory.

Yeah.  Looking at the gcc source, the option has actually been around
since 2000.  So, never mind!

I'd be ok with v1, plus a comment saying that clang doesn't support the
option.

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1636812 — [Patch v3] x86/build: don't add -maccumulate-outgoing-args w/o compiler support

FromNick Desaulniers <nick.desaulniers@gmail.com>
Date2017-05-06 05:10 +0200
Subject[Patch v3] x86/build: don't add -maccumulate-outgoing-args w/o compiler support
Message-ID<tDYgx-2vP-1@gated-at.bofh.it>
In reply to#1636366
Clang does not support this machine dependent option.

Older versions of GCC (pre 3.0) may not support this option, added in
2000, but it's unlikely they can still compile the kernel.

Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
---
 arch/x86/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 4430dd489620..5a0ac8411792 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -179,7 +179,7 @@ ifdef CONFIG_JUMP_LABEL
 endif
 
 ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
-	KBUILD_CFLAGS += -maccumulate-outgoing-args
+	KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args,)
 endif
 
 # Stackpointer is addressed different for 32 bit and 64 bit x86
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1637195 — Re: [Patch v3] x86/build: don't add -maccumulate-outgoing-args w/o compiler support

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-05-08 05:30 +0200
SubjectRe: [Patch v3] x86/build: don't add -maccumulate-outgoing-args w/o compiler support
Message-ID<tEHx0-6IQ-3@gated-at.bofh.it>
In reply to#1636812
On Fri, May 05, 2017 at 08:05:09PM -0700, Nick Desaulniers wrote:
> Clang does not support this machine dependent option.
> 
> Older versions of GCC (pre 3.0) may not support this option, added in
> 2000, but it's unlikely they can still compile the kernel.
> 
> Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
> ---
>  arch/x86/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 4430dd489620..5a0ac8411792 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -179,7 +179,7 @@ ifdef CONFIG_JUMP_LABEL
>  endif
>  
>  ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
> -	KBUILD_CFLAGS += -maccumulate-outgoing-args
> +	KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args,)
>  endif
>  
>  # Stackpointer is addressed different for 32 bit and 64 bit x86

It would be useful to add a comment in the Makefile above the
KBUILD_CFLAGS line stating that clang doesn't support this flag.

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1637852 — [Patch v4] x86/build: don't add -maccumulate-outgoing-args w/o compiler support

FromNick Desaulniers <nick.desaulniers@gmail.com>
Date2017-05-09 05:40 +0200
Subject[Patch v4] x86/build: don't add -maccumulate-outgoing-args w/o compiler support
Message-ID<tF4ad-4v9-5@gated-at.bofh.it>
In reply to#1637195
Clang does not support this machine dependent option.

Older versions of GCC (pre 3.0) may not support this option, added in
2000, but it's unlikely they can still compile the kernel.

Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
---
 arch/x86/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 4430dd489620..8c4dcba9a6a7 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -179,7 +179,8 @@ ifdef CONFIG_JUMP_LABEL
 endif
 
 ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
-	KBUILD_CFLAGS += -maccumulate-outgoing-args
+	# Unsupported by Clang.
+	KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args,)
 endif
 
 # Stackpointer is addressed different for 32 bit and 64 bit x86
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1637853 — Re: [Patch v4] x86/build: don't add -maccumulate-outgoing-args w/o compiler support

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-05-09 05:50 +0200
SubjectRe: [Patch v4] x86/build: don't add -maccumulate-outgoing-args w/o compiler support
Message-ID<tF4jT-4yr-1@gated-at.bofh.it>
In reply to#1637852
On Mon, May 08, 2017 at 08:29:46PM -0700, Nick Desaulniers wrote:
> Clang does not support this machine dependent option.
> 
> Older versions of GCC (pre 3.0) may not support this option, added in
> 2000, but it's unlikely they can still compile the kernel.
> 
> Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
> ---
>  arch/x86/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 4430dd489620..8c4dcba9a6a7 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -179,7 +179,8 @@ ifdef CONFIG_JUMP_LABEL
>  endif
>  
>  ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
> -	KBUILD_CFLAGS += -maccumulate-outgoing-args
> +	# Unsupported by Clang.
> +	KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args,)
>  endif
>  
>  # Stackpointer is addressed different for 32 bit and 64 bit x86

Looks good to me, thanks for the iterations.

Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1637899 — [tip:x86/urgent] x86/build: Don't add -maccumulate-outgoing-args w/o compiler support

Fromtip-bot for Nick Desaulniers <tipbot@zytor.com>
Date2017-05-09 08:50 +0200
Subject[tip:x86/urgent] x86/build: Don't add -maccumulate-outgoing-args w/o compiler support
Message-ID<tF786-6qs-19@gated-at.bofh.it>
In reply to#1637852
Commit-ID:  4a1bec4605838fd7872ec41677585e241e256785
Gitweb:     http://git.kernel.org/tip/4a1bec4605838fd7872ec41677585e241e256785
Author:     Nick Desaulniers <nick.desaulniers@gmail.com>
AuthorDate: Mon, 8 May 2017 20:29:46 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 9 May 2017 08:16:45 +0200

x86/build: Don't add -maccumulate-outgoing-args w/o compiler support

Clang does not support this machine dependent option.

Older versions of GCC (pre 3.0) may not support this option, added in
2000, but it's unlikely they can still compile a working kernel.

Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20170509032946.20444-1-nick.desaulniers@gmail.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 4430dd4..5851411 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -179,7 +179,8 @@ ifdef CONFIG_JUMP_LABEL
 endif
 
 ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
-	KBUILD_CFLAGS += -maccumulate-outgoing-args
+	# This compiler flag is not supported by Clang:
+	KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args,)
 endif
 
 # Stackpointer is addressed different for 32 bit and 64 bit x86

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web