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


Groups > linux.kernel > #1671429

Re: [PATCH v4 3/3] x86/build: Specify stack alignment for clang

From Ingo Molnar <mingo@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH v4 3/3] x86/build: Specify stack alignment for clang
Date 2017-06-21 09:20 +0200
Message-ID <tUI5J-31b-39@gated-at.bofh.it> (permalink)
References (1 earlier) <tUa41-6pD-21@gated-at.bofh.it> <tUbCO-7vX-23@gated-at.bofh.it> <tUbMu-7Ag-17@gated-at.bofh.it> <tUnDY-6Ll-23@gated-at.bofh.it> <tUvia-3ak-15@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


* Matthias Kaehlcke <mka@chromium.org> wrote:

> El Tue, Jun 20, 2017 at 11:20:54AM +0200 Ingo Molnar ha dit:
> 
> > 
> > * Matthias Kaehlcke <mka@chromium.org> wrote:
> > 
> > > Ingo didn't like the duplication and suggested the use of a variable, which 
> > > kinda implies a check for the compiler name.
> > 
> > I don't think it implies that: why cannot cc_stack_align_opt probe for the 
> > compiler option and use whichever is available, without hard-coding the compiler 
> > name?
> 
> We could do this:
> 
> ifneq ($(call __cc-option, $(CC), -mno-sse, -mpreferred-stack-boundary=3,),)
>         cc_stack_align_opt := -mpreferred-stack-boundary
> endif
> ifneq ($(call cc-option, -mstack-alignment=3,),)
>         cc_stack_align_opt := -mstack-alignment
> endif

The principle Looks good to me - but I'd make the second probing an 'else' branch, 
i.e. probe for a suitable compiler option until we find one. That would also not 
burden the GCC build with probing for different compiler options.

Please also add a comment in the code that explains that the first option is a GCC 
option and the second one is a Clang-ism.

> Since this solution also won't win a beauty price please let me know
> if it is acceptable before respinning the patch or if you have other
> suggestions.

This one already looks a lot cleaner to me than any of the previous ones.

Thanks,

	Ingo

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

Re: [PATCH v4 3/3] x86/build: Specify stack alignment for clang Ingo Molnar <mingo@kernel.org> - 2017-06-20 11:30 +0200
  Re: [PATCH v4 3/3] x86/build: Specify stack alignment for clang Matthias Kaehlcke <mka@chromium.org> - 2017-06-20 19:40 +0200
    Re: [PATCH v4 3/3] x86/build: Specify stack alignment for clang Ingo Molnar <mingo@kernel.org> - 2017-06-21 09:20 +0200

csiph-web