On Fri, 2015-01-23 at 08:29 +0000, Jan Beulich wrote:
> Just like for AVX2 (which simply needs an #if -> #ifdef conversion),
> SSSE3 assembler support should be checked for before using it.
>
> Signed-off-by: Jan Beulich <[email protected]>
> Cc: Jim Kukunas <[email protected]>
> Cc: Neil Brown <[email protected]>
This patch became commit be46ac86a81b ("x86/raid6: correctly check for
assembler capabilities") in today's linux-next (ie, next-20150203). I
noticed because a script I use to check linux-next spotted a potential
problem with it.
> ---
> arch/x86/Makefile | 1 +
> lib/raid6/algos.c | 2 +-
> lib/raid6/recov_avx2.c | 2 +-
> lib/raid6/recov_ssse3.c | 6 ++++++
> 4 files changed, 9 insertions(+), 2 deletions(-)
>
> --- 3.19-rc5/arch/x86/Makefile
> +++ 3.19-rc5-raid6-SSSE3/arch/x86/Makefile
> @@ -148,6 +148,7 @@ cfi-sections := $(call as-instr,.cfi_sec
>
> # does binutils support specific instructions?
> asinstr := $(call as-instr,fxsaveq (%rax),-DCONFIG_AS_FXSAVEQ=1)
> +asinstr += $(call as-instr,pshufb %xmm0$(comma)%xmm0,-DCONFIG_AS_SSSE3=1)
This Makefile defines a preprocessor macro with a CONFIG_ prefix. Almost
all macros with that prefix are defined through the kconfig system. A
handful, like CONFIG_AS_SSSE3 and a few other macros defined in this
Makefile, are not. Apparently this is a pet peeve I share with few
people, but would any other prefix than CONFIG_ work for you too?
> asinstr += $(call as-instr,crc32l %eax$(comma)%eax,-DCONFIG_AS_CRC32=1)
> avx_instr := $(call as-instr,vxorps %ymm0$(comma)%ymm1$(comma)%ymm2,-DCONFIG_AS_AVX=1)
> avx2_instr :=$(call as-instr,vpbroadcastb %xmm0$(comma)%ymm1,-DCONFIG_AS_AVX2=1)
Paul Bolle
On Tue, 03 Feb 2015 21:31:30 +0100 Paul Bolle <[email protected]> wrote:
> On Fri, 2015-01-23 at 08:29 +0000, Jan Beulich wrote:
> > Just like for AVX2 (which simply needs an #if -> #ifdef conversion),
> > SSSE3 assembler support should be checked for before using it.
> >
> > Signed-off-by: Jan Beulich <[email protected]>
> > Cc: Jim Kukunas <[email protected]>
> > Cc: Neil Brown <[email protected]>
>
> This patch became commit be46ac86a81b ("x86/raid6: correctly check for
> assembler capabilities") in today's linux-next (ie, next-20150203). I
> noticed because a script I use to check linux-next spotted a potential
> problem with it.
>
> > ---
> > arch/x86/Makefile | 1 +
> > lib/raid6/algos.c | 2 +-
> > lib/raid6/recov_avx2.c | 2 +-
> > lib/raid6/recov_ssse3.c | 6 ++++++
> > 4 files changed, 9 insertions(+), 2 deletions(-)
> >
> > --- 3.19-rc5/arch/x86/Makefile
> > +++ 3.19-rc5-raid6-SSSE3/arch/x86/Makefile
> > @@ -148,6 +148,7 @@ cfi-sections := $(call as-instr,.cfi_sec
> >
> > # does binutils support specific instructions?
> > asinstr := $(call as-instr,fxsaveq (%rax),-DCONFIG_AS_FXSAVEQ=1)
> > +asinstr += $(call as-instr,pshufb %xmm0$(comma)%xmm0,-DCONFIG_AS_SSSE3=1)
>
> This Makefile defines a preprocessor macro with a CONFIG_ prefix. Almost
> all macros with that prefix are defined through the kconfig system. A
> handful, like CONFIG_AS_SSSE3 and a few other macros defined in this
> Makefile, are not. Apparently this is a pet peeve I share with few
> people, but would any other prefix than CONFIG_ work for you too?
Actually the prefix of this macro is "CONFIG_AS_", not "CONFIG_" :-)
CONFIG_AS_ is reserved for assembly magic, and is never used by the the
kconfig system.
(Well..... I might have made bits of that up, but "git grep 'config AS_'"
doesn't find anything).
NeilBrown
>
> > asinstr += $(call as-instr,crc32l %eax$(comma)%eax,-DCONFIG_AS_CRC32=1)
> > avx_instr := $(call as-instr,vxorps %ymm0$(comma)%ymm1$(comma)%ymm2,-DCONFIG_AS_AVX=1)
> > avx2_instr :=$(call as-instr,vpbroadcastb %xmm0$(comma)%ymm1,-DCONFIG_AS_AVX2=1)
>
>
> Paul Bolle
On Wed, 2015-02-04 at 07:50 +1100, NeilBrown wrote:
> Actually the prefix of this macro is "CONFIG_AS_", not "CONFIG_" :-)
> CONFIG_AS_ is reserved for assembly magic, and is never used by the the
> kconfig system.
>
> (Well..... I might have made bits of that up, but "git grep 'config AS_'"
> doesn't find anything).
That's correct, there are no Kconfig symbols starting with AS_. But
still, I would like to hear whether there's a reasonable chance I might
convince other people to adopt my peeve.
The thinking behind that peeve is, basically, that where people
encounter a CONFIG_* macro they should only have to check the .config
file to see how that macro was evaluated in the build that was used.
Thanks,
Paul Bolle
On Tue, 03 Feb 2015 22:03:35 +0100 Paul Bolle <[email protected]> wrote:
> On Wed, 2015-02-04 at 07:50 +1100, NeilBrown wrote:
> > Actually the prefix of this macro is "CONFIG_AS_", not "CONFIG_" :-)
> > CONFIG_AS_ is reserved for assembly magic, and is never used by the the
> > kconfig system.
> >
> > (Well..... I might have made bits of that up, but "git grep 'config AS_'"
> > doesn't find anything).
>
> That's correct, there are no Kconfig symbols starting with AS_. But
> still, I would like to hear whether there's a reasonable chance I might
> convince other people to adopt my peeve.
>
> The thinking behind that peeve is, basically, that where people
> encounter a CONFIG_* macro they should only have to check the .config
> file to see how that macro was evaluated in the build that was used.
>
Personally, I don't care.
But I find that developers in general are more responsive to code than to
peeves.
So if you post a patch which makes the change that you want, then you are
more likely to get a useful response than if you just post a peeve.
It may not be the response you want of course....
NeilBrown
On Tue, Feb 3, 2015 at 10:09 PM, NeilBrown <[email protected]> wrote:
> On Tue, 03 Feb 2015 22:03:35 +0100 Paul Bolle <[email protected]> wrote:
>
>> On Wed, 2015-02-04 at 07:50 +1100, NeilBrown wrote:
>> > Actually the prefix of this macro is "CONFIG_AS_", not "CONFIG_" :-)
>> > CONFIG_AS_ is reserved for assembly magic, and is never used by the the
>> > kconfig system.
>> >
>> > (Well..... I might have made bits of that up, but "git grep 'config AS_'"
>> > doesn't find anything).
>>
>> That's correct, there are no Kconfig symbols starting with AS_. But
>> still, I would like to hear whether there's a reasonable chance I might
>> convince other people to adopt my peeve.
>>
>> The thinking behind that peeve is, basically, that where people
>> encounter a CONFIG_* macro they should only have to check the .config
>> file to see how that macro was evaluated in the build that was used.
>>
>
> Personally, I don't care.
A problem with those identifiers is that the CONFIG_ prefix is
reserved for Kconfig features in Make and CPP syntax. The _MODULE
suffix for CPP alone. Sadly, this convention is only documented in
the Kconfig C code itself. Nonetheless, such cases give hard times to
static analysis tools that then have to deal with such false
positives.
Kind regards,
Valentin
> But I find that developers in general are more responsive to code than to
> peeves.
>
> So if you post a patch which makes the change that you want, then you are
> more likely to get a useful response than if you just post a peeve.
> It may not be the response you want of course....
>
> NeilBrown
>>> On 03.02.15 at 22:03, <[email protected]> wrote:
> On Wed, 2015-02-04 at 07:50 +1100, NeilBrown wrote:
>> Actually the prefix of this macro is "CONFIG_AS_", not "CONFIG_" :-)
>> CONFIG_AS_ is reserved for assembly magic, and is never used by the the
>> kconfig system.
>>
>> (Well..... I might have made bits of that up, but "git grep 'config AS_'"
>> doesn't find anything).
>
> That's correct, there are no Kconfig symbols starting with AS_. But
> still, I would like to hear whether there's a reasonable chance I might
> convince other people to adopt my peeve.
>
> The thinking behind that peeve is, basically, that where people
> encounter a CONFIG_* macro they should only have to check the .config
> file to see how that macro was evaluated in the build that was used.
I too found it curious that CONFIG_* is being used here instead of
e.g. HAVE_*, but with the patch at hand I simply followed suit (as
that normally is the route involving less discussions in order to get
a necessary/desirable fix accepted). In the end I guess you'd need
to convince the x86 maintainers.
Jan
On 02/03/2015 01:03 PM, Paul Bolle wrote:
> On Wed, 2015-02-04 at 07:50 +1100, NeilBrown wrote:
>> Actually the prefix of this macro is "CONFIG_AS_", not "CONFIG_" :-)
>> CONFIG_AS_ is reserved for assembly magic, and is never used by the the
>> kconfig system.
>>
>> (Well..... I might have made bits of that up, but "git grep 'config AS_'"
>> doesn't find anything).
>
> That's correct, there are no Kconfig symbols starting with AS_. But
> still, I would like to hear whether there's a reasonable chance I might
> convince other people to adopt my peeve.
>
> The thinking behind that peeve is, basically, that where people
> encounter a CONFIG_* macro they should only have to check the .config
> file to see how that macro was evaluated in the build that was used.
>
There is a hope/intent that eventually the config system will be able to
incorporate toolchain dependencies for a bunch of reasons.
-hpa
On Tue, 2015-06-16 at 12:46 -0700, H. Peter Anvin wrote:
> There is a hope/intent that eventually the config system will be able to
> incorporate toolchain dependencies for a bunch of reasons.
This restarts a four months old thread with a one sentence remark. So
could you please elaborate, because now you've left me and, perhaps, the
other people reading this wondering what "toolchain dependencies"
actually means and what those "bunch of reasons" are.
Thanks,
Paul Bolle
On 06/16/2015 12:56 PM, Paul Bolle wrote:
> On Tue, 2015-06-16 at 12:46 -0700, H. Peter Anvin wrote:
>> There is a hope/intent that eventually the config system will be able to
>> incorporate toolchain dependencies for a bunch of reasons.
>
> This restarts a four months old thread with a one sentence remark. So
> could you please elaborate, because now you've left me and, perhaps, the
> other people reading this wondering what "toolchain dependencies"
> actually means and what those "bunch of reasons" are.
>
Sorry, missed the date on this in my inbox for some reason.
So this related to the CONFIG_AS_ symbols for assembly. We really would
like to do things like actually adding dependencies on assembler or
compiler support into Kconfig proper, rather than having two independent
mechanisms. That way we could do, for example:
config RAID6_AVX2
depends on X86 && AS_AVX2
On Tue, 2015-06-16 at 13:05 -0700, H. Peter Anvin wrote:
> So this related to the CONFIG_AS_ symbols for assembly. We really would
> like to do things like actually adding dependencies on assembler or
> compiler support into Kconfig proper, rather than having two independent
> mechanisms. That way we could do, for example:
>
> config RAID6_AVX2
> depends on X86 && AS_AVX2
I have no idea what AS_AVX2 means, sorry. So what would it mean if
someone has a .config with
CONFIG_AS_AVX2=foo
Ie, what would happen if someone using a toolchain for which that would
not be possible tries to build a kernel with that symbol set? That
person's .config would end up containing
# CONFIG_AS_AVX2 is not set
or similar, right?
Paul Bolle
On 06/16/2015 01:37 PM, Paul Bolle wrote:
>
> I have no idea what AS_AVX2 means, sorry. So what would it mean if
> someone has a .config with
> CONFIG_AS_AVX2=foo
>
> Ie, what would happen if someone using a toolchain for which that would
> not be possible tries to build a kernel with that symbol set? That
> person's .config would end up containing
> # CONFIG_AS_AVX2 is not set
>
> or similar, right?
>
Yes, these would be generated, not user input.
-hpa
On Tue, 2015-06-16 at 13:42 -0700, H. Peter Anvin wrote:
> Yes, these would be generated, not user input.
I think someone (Jiri Kosina?) proposed shelling out to set Kconfig
values. I don't think any patches were ever submitted. But I guess
something along those lines would be needed to adopt the .config to the
toolchain at hand. As long as we don't do anything funny once we've
generated the .config file that might just work.
Yes, I know, the devil hides in the details.
Thanks,
Paul Bolle