cmake: read asm flags correctly from environment in toolchain file

Message ID 20220214153807.21216-1-martin.beeger@online.de
State New
Headers show
Series cmake: read asm flags correctly from environment in toolchain file | expand

Commit Message

Martin Beeger Feb. 14, 2022, 3:38 p.m. UTC
As discussied in [YOCTO #14717] cmake contains a OEToolchainConfig.cmake
file to configure the toolchain correctly in cross-compile build for recipes
using cmake. The CMAKE_ASM_FLAGS are the configuration for inline assembly,
and these are set in toolchain environment via the ASMFLAGS variable.
This changes the toolchain so cmake correctly picks up the given ASMFLAGS
instead of errorneously forcing the C flags instead.

Signed-off-by: Martin Beeger <martin.beeger@online.de>
---
 meta/recipes-devtools/cmake/cmake/OEToolchainConfig.cmake | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Richard Purdie Feb. 14, 2022, 3:43 p.m. UTC | #1
On Mon, 2022-02-14 at 16:38 +0100, Martin Beeger wrote:
> As discussied in [YOCTO #14717] cmake contains a OEToolchainConfig.cmake
> file to configure the toolchain correctly in cross-compile build for recipes
> using cmake. The CMAKE_ASM_FLAGS are the configuration for inline assembly,
> and these are set in toolchain environment via the ASMFLAGS variable.
> This changes the toolchain so cmake correctly picks up the given ASMFLAGS
> instead of errorneously forcing the C flags instead.
> 
> Signed-off-by: Martin Beeger <martin.beeger@online.de>
> ---
>  meta/recipes-devtools/cmake/cmake/OEToolchainConfig.cmake | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-devtools/cmake/cmake/OEToolchainConfig.cmake b/meta/recipes-devtools/cmake/cmake/OEToolchainConfig.cmake
> index 86446c3ace..3513d05b2f 100644
> --- a/meta/recipes-devtools/cmake/cmake/OEToolchainConfig.cmake
> +++ b/meta/recipes-devtools/cmake/cmake/OEToolchainConfig.cmake
> @@ -1,7 +1,7 @@
>  set( CMAKE_SYSTEM_NAME Linux )
>  set( CMAKE_C_FLAGS $ENV{CFLAGS} CACHE STRING "" FORCE )
>  set( CMAKE_CXX_FLAGS $ENV{CXXFLAGS}  CACHE STRING "" FORCE )
> -set( CMAKE_ASM_FLAGS ${CMAKE_C_FLAGS} CACHE STRING "" FORCE )
> +set( CMAKE_ASM_FLAGS $ENV{ASMFLAGS} CACHE STRING "" FORCE )
>  set( CMAKE_SYSROOT $ENV{OECORE_TARGET_SYSROOT} )
>  
>  set( CMAKE_FIND_ROOT_PATH $ENV{OECORE_TARGET_SYSROOT} )

I'm a little more puzzled on this one since whilst we set CFLAGS and CXXFLAGS in
bitbake.conf, we don't set ASMFLAGS and it wouldn't be present in our
environment.

Perhaps this should be $ENV{CFLAGS} instead of ${CMAKE_C_FLAGS}? There are flags
in our CFLAGS which like need to be passed to the assembler too?

Cheers,

Richard
Martin Beeger Feb. 14, 2022, 4:32 p.m. UTC | #2
> I'm a little more puzzled on this one since whilst we set CFLAGS and CXXFLAGS in
> bitbake.conf, we don't set ASMFLAGS and it wouldn't be present in our
> environment.
>
> Perhaps this should be $ENV{CFLAGS} instead of ${CMAKE_C_FLAGS}? There are flags
> in our CFLAGS which like need to be passed to the assembler too?

I am a bit unsure now about this.
As far as I understand the environment-setup will not set ASMFLAGS, that 
is correct. For all non-cmake recipes, the compiler will not pick up 
CFLAGS instead. It will simply still use no extra parameters for the 
inline assembly.
In cmake it will use the CFLAGS for the ASM too, yielding inconsistent 
behaviour over build systems. Is that really a good thing?

Also the CFLAGS are usually "-O2 -g". At least the -g parameter will 
have limited effect for the assembly, and optimizations for inline 
assembly are also extremely limited.
So the practical difference between both modes and whether the flags 
propagate through is minimal, if at all existent.

All critical compiler parameters like the --sysroot and others are not 
stored in CFLAGS, but are instead directly appended to the compiler 
executable path for CC, CXX and so on. These are always applied and 
cmake does understand these, parses them out, forces them to always be 
there etc.
This code has nothing to do with that, it only impacts the optimization 
and debugging flags.

This patch makes this behaviour consistent between cmake and non-cmake 
recipes in that is never applies ASM flags, instead of inconsistently 
applying them.
Peter Kjellerstedt Feb. 15, 2022, 2:18 p.m. UTC | #3
> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Martin Beeger
> Sent: den 14 februari 2022 17:32
> To: Richard Purdie <richard.purdie@linuxfoundation.org>; openembedded-
> core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH] cmake: read asm flags correctly from
> environment in toolchain file
> 
> 
> > I'm a little more puzzled on this one since whilst we set CFLAGS and
> CXXFLAGS in
> > bitbake.conf, we don't set ASMFLAGS and it wouldn't be present in our
> > environment.
> >
> > Perhaps this should be $ENV{CFLAGS} instead of ${CMAKE_C_FLAGS}? There
> are flags
> > in our CFLAGS which like need to be passed to the assembler too?
> 
> I am a bit unsure now about this.
> As far as I understand the environment-setup will not set ASMFLAGS, that
> is correct. For all non-cmake recipes, the compiler will not pick up
> CFLAGS instead. It will simply still use no extra parameters for the
> inline assembly.
> In cmake it will use the CFLAGS for the ASM too, yielding inconsistent
> behaviour over build systems. Is that really a good thing?
> 
> Also the CFLAGS are usually "-O2 -g". At least the -g parameter will
> have limited effect for the assembly, and optimizations for inline
> assembly are also extremely limited.
> So the practical difference between both modes and whether the flags
> propagate through is minimal, if at all existent.
> 
> All critical compiler parameters like the --sysroot and others are not
> stored in CFLAGS, but are instead directly appended to the compiler
> executable path for CC, CXX and so on. These are always applied and
> cmake does understand these, parses them out, forces them to always be
> there etc.
> This code has nothing to do with that, it only impacts the optimization
> and debugging flags.
> 
> This patch makes this behaviour consistent between cmake and non-cmake
> recipes in that is never applies ASM flags, instead of inconsistently
> applying them.

For the record, the variable name that, e.g., make expects for assembler 
flags in its default rules is "ASFLAGS", not "ASMFLAGS".

//Peter
Martin Beeger Feb. 15, 2022, 8:25 p.m. UTC | #4
The docs say that cmake is consuming ASMFLAGS 
(https://cmake.org/cmake/help/v3.23/envvar/ASM_DIALECTFLAGS.html). If 
that is not the correct spelling for the given langage mode for gcc, we 
should probably set CMAKE_AS_FLAGS instead and assume the general 
mechanism in 
https://cmake.org/cmake/help/v3.23/variable/CMAKE_LANG_FLAGS.html works.
It seems the best option to not provide the line at all in this case,
as the default set here is unlikely to be correct then.

Are there any projects/recipes who are known to use cmake in combination 
with providing direct assembly files and linking them to programs (in 
openembedded)?





Am 15.02.22 um 15:18 schrieb Peter Kjellerstedt:
>> -----Original Message-----
>> From: openembedded-core@lists.openembedded.org <openembedded-
>> core@lists.openembedded.org> On Behalf Of Martin Beeger
>> Sent: den 14 februari 2022 17:32
>> To: Richard Purdie <richard.purdie@linuxfoundation.org>; openembedded-
>> core@lists.openembedded.org
>> Subject: Re: [OE-core] [PATCH] cmake: read asm flags correctly from
>> environment in toolchain file
>>
>>
>>> I'm a little more puzzled on this one since whilst we set CFLAGS and
>> CXXFLAGS in
>>> bitbake.conf, we don't set ASMFLAGS and it wouldn't be present in our
>>> environment.
>>>
>>> Perhaps this should be $ENV{CFLAGS} instead of ${CMAKE_C_FLAGS}? There
>> are flags
>>> in our CFLAGS which like need to be passed to the assembler too?
>>
>> I am a bit unsure now about this.
>> As far as I understand the environment-setup will not set ASMFLAGS, that
>> is correct. For all non-cmake recipes, the compiler will not pick up
>> CFLAGS instead. It will simply still use no extra parameters for the
>> inline assembly.
>> In cmake it will use the CFLAGS for the ASM too, yielding inconsistent
>> behaviour over build systems. Is that really a good thing?
>>
>> Also the CFLAGS are usually "-O2 -g". At least the -g parameter will
>> have limited effect for the assembly, and optimizations for inline
>> assembly are also extremely limited.
>> So the practical difference between both modes and whether the flags
>> propagate through is minimal, if at all existent.
>>
>> All critical compiler parameters like the --sysroot and others are not
>> stored in CFLAGS, but are instead directly appended to the compiler
>> executable path for CC, CXX and so on. These are always applied and
>> cmake does understand these, parses them out, forces them to always be
>> there etc.
>> This code has nothing to do with that, it only impacts the optimization
>> and debugging flags.
>>
>> This patch makes this behaviour consistent between cmake and non-cmake
>> recipes in that is never applies ASM flags, instead of inconsistently
>> applying them.
> 
> For the record, the variable name that, e.g., make expects for assembler
> flags in its default rules is "ASFLAGS", not "ASMFLAGS".
> 
> //Peter
>
Martin Beeger July 18, 2022, 4:47 p.m. UTC | #5
I looked further, and the variable is misspelt and should be CMAKE_AS_FLAGS to have any effect. Furthermore, it is not recommended to set it explicitly, as cmake will initialize it to the correct default. If the toolchain file from open embedded in general contains ASFLGS, cmake wills et that as CMAKE_AS_FLAGS automatically and pass those parameters on.

This variable did not change this behaviour because it was spelt differently, although I assume the author intended to do that.
I would still suggest removing this variable. (as this patch does)
Martin Beeger July 18, 2022, 4:48 p.m. UTC | #6
oh wait, I failed to send v2 of this patch. Will do so.

Patch

diff --git a/meta/recipes-devtools/cmake/cmake/OEToolchainConfig.cmake b/meta/recipes-devtools/cmake/cmake/OEToolchainConfig.cmake
index 86446c3ace..3513d05b2f 100644
--- a/meta/recipes-devtools/cmake/cmake/OEToolchainConfig.cmake
+++ b/meta/recipes-devtools/cmake/cmake/OEToolchainConfig.cmake
@@ -1,7 +1,7 @@ 
 set( CMAKE_SYSTEM_NAME Linux )
 set( CMAKE_C_FLAGS $ENV{CFLAGS} CACHE STRING "" FORCE )
 set( CMAKE_CXX_FLAGS $ENV{CXXFLAGS}  CACHE STRING "" FORCE )
-set( CMAKE_ASM_FLAGS ${CMAKE_C_FLAGS} CACHE STRING "" FORCE )
+set( CMAKE_ASM_FLAGS $ENV{ASMFLAGS} CACHE STRING "" FORCE )
 set( CMAKE_SYSROOT $ENV{OECORE_TARGET_SYSROOT} )
 
 set( CMAKE_FIND_ROOT_PATH $ENV{OECORE_TARGET_SYSROOT} )