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

Groups > linux.kernel > #1670522 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  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

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

* 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?

> I also think this is a cleaner solution. [...]

I concur with hpa: hard-coding compiler is awfully fragile and ugly as well.

With the proper probing of compiler options it will be possible for compilers to 
consolidate their options, and it would be possible for a third compiler to use a 
mixture of GCC and Clang options. With hard-coding none of that flexibility is 
available.

> but I'm happy to respin the patch if you have another suggestion that is ok for 
> both of you.

Please do.

Thanks,

	Ingo

[toc] | [next] | [standalone]

#1670987

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

If preferred cc-option could be used to probe for
-mpreferred-stack-boundary , however it would require REALMODE_CFLAGS
to be moved further down in the Makefile.

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.

> > I also think this is a cleaner solution. [...]
> 
> I concur with hpa: hard-coding compiler is awfully fragile and ugly as well.
> 
> With the proper probing of compiler options it will be possible for compilers to 
> consolidate their options, and it would be possible for a third compiler to use a 
> mixture of GCC and Clang options. With hard-coding none of that flexibility is 
> available.
> 
> > but I'm happy to respin the patch if you have another suggestion that is ok for 
> > both of you.
> 
> Please do.
> 
> Thanks,
> 
> 	Ingo

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

#1671429

* 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web