Skip to contentSkip to conversation

pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR.

PATCH Superseded Volker Krause 3 participants 5 messages Nov 16, 2014 – Nov 18, 2014

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR.

This is rarely needed by packages, but convenient to have when it is.

---
Changes v1 -> v2:
- added missing qstrip
- set the correct ARM variant as done in

    http://patchwork.ozlabs.org/patch/408501/
 package/pkg-cmake.mk                | 15 +++++++++++++++
 support/misc/toolchainfile.cmake.in |  1 +
 2 files changed, 16 insertions(+)
package/pkg-cmake.mk +15 -0
index c8735ef..619da9b 100644
@@ -180,6 +180,20 @@ host-cmake-package = $(call inner-cmake-package,host-$(pkgname),$(call UPPERCASE
 # Generation of the CMake toolchain file
 ################################################################################
 
+# CMAKE_SYSTEM_PROCESSOR should match uname -m
+ifeq ($(BR2_ARM_CPU_ARMV5),y)
+CMAKE_SYSTEM_PROCESSOR = armv5
+endif
+ifeq ($(BR2_ARM_CPU_ARMV6),y)
+CMAKE_SYSTEM_PROCESSOR = armv6l
+endif
+ifeq ($(BR2_ARM_CPU_ARMV7A),y)
+CMAKE_SYSTEM_PROCESSOR = armv7l
+endif
+ifndef CMAKE_SYSTEM_PROCESSOR
+CMAKE_SYSTEM_PROCESSOR = $(BR2_ARCH)
+endif
+
 # In order to allow the toolchain to be relocated, we calculate the HOST_DIR
 # based on the toolchainfile.cmake file's location: $(HOST_DIR)/usr/share/buildroot
 # In all the other variables, HOST_DIR will be replaced by RELOCATED_HOST_DIR,
@@ -193,5 +207,6 @@ $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake:
 		-e 's:@@TARGET_LDFLAGS@@:$(call qstrip,$(TARGET_LDFLAGS)):' \
 		-e 's:@@TARGET_CC_NOCCACHE@@:$(subst $(HOST_DIR)/,,$(call qstrip,$(TARGET_CC_NOCCACHE))):' \
 		-e 's:@@TARGET_CXX_NOCCACHE@@:$(subst $(HOST_DIR)/,,$(call qstrip,$(TARGET_CXX_NOCCACHE))):' \
+		-e 's:@@CMAKE_SYSTEM_PROCESSOR@@:$(call qstrip,$(CMAKE_SYSTEM_PROCESSOR)):' \
 		$(TOPDIR)/support/misc/toolchainfile.cmake.in \
 		> $@
support/misc/toolchainfile.cmake.in +1 -0
index 4ca3d35..816af13 100644
@@ -11,6 +11,7 @@
 string(REPLACE "/usr/share/buildroot" "" RELOCATED_HOST_DIR ${CMAKE_CURRENT_LIST_DIR})
 
 set(CMAKE_SYSTEM_NAME Linux)
+set(CMAKE_SYSTEM_PROCESSOR @@CMAKE_SYSTEM_PROCESSOR@@)
 
 set(CMAKE_C_FLAGS "@@TARGET_CFLAGS@@ ${CMAKE_C_FLAGS}" CACHE STRING "Buildroot CFLAGS")
 set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS")
Signature · 1 line
1.8.4.5
#1 lore raw

PATCH v2 1/1 pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR.

Volker, all,

Quoted 18 lines · Volker Krause

This is rarely needed by packages, but convenient to have when it is.

---
Changes v1 -> v2:
- added missing qstrip
- set the correct ARM variant as done in

    http://patchwork.ozlabs.org/patch/408501/
 package/pkg-cmake.mk                | 15 +++++++++++++++
 support/misc/toolchainfile.cmake.in |  1 +
 2 files changed, 16 insertions(+)
package/pkg-cmake.mk +0 -0
index c8735ef..619da9b 100644

package/pkg-cmake.mk host-cmake-package = $(call inner-cmake-package,host-$(pkgname),$(call UPPERCASE · line 185

Quoted 6 lines
package/pkg-cmake.mk +3 -0
@@ -180,6 +180,20 @@ host-cmake-package = $(call inner-cmake-package,host-$(pkgname),$(call UPPERCASE
 # Generation of the CMake toolchain file
 ################################################################################
 
+# CMAKE_SYSTEM_PROCESSOR should match uname -m
+ifeq ($(BR2_ARM_CPU_ARMV5),y)
+CMAKE_SYSTEM_PROCESSOR = armv5

I'm not a big fan of the name of this variable...
Let's see what others think about it.

Quoted 34 lines
package/pkg-cmake.mk +12 -0
+endif
+ifeq ($(BR2_ARM_CPU_ARMV6),y)
+CMAKE_SYSTEM_PROCESSOR = armv6l
+endif
+ifeq ($(BR2_ARM_CPU_ARMV7A),y)
+CMAKE_SYSTEM_PROCESSOR = armv7l
+endif
+ifndef CMAKE_SYSTEM_PROCESSOR
+CMAKE_SYSTEM_PROCESSOR = $(BR2_ARCH)
+endif
+
 # In order to allow the toolchain to be relocated, we calculate the HOST_DIR
 # based on the toolchainfile.cmake file's location: $(HOST_DIR)/usr/share/buildroot
 # In all the other variables, HOST_DIR will be replaced by RELOCATED_HOST_DIR,
@@ -193,5 +207,6 @@ $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake:
                -e 's:@@TARGET_LDFLAGS@@:$(call qstrip,$(TARGET_LDFLAGS)):' \
                -e 's:@@TARGET_CC_NOCCACHE@@:$(subst $(HOST_DIR)/,,$(call qstrip,$(TARGET_CC_NOCCACHE))):' \
                -e 's:@@TARGET_CXX_NOCCACHE@@:$(subst $(HOST_DIR)/,,$(call qstrip,$(TARGET_CXX_NOCCACHE))):' \
+               -e 's:@@CMAKE_SYSTEM_PROCESSOR@@:$(call qstrip,$(CMAKE_SYSTEM_PROCESSOR)):' \
                $(TOPDIR)/support/misc/toolchainfile.cmake.in \
                > $@
support/misc/toolchainfile.cmake.in +1 -1
index 4ca3d35..816af13 100644
@@ -11,6 +11,7 @@
 string(REPLACE "/usr/share/buildroot" "" RELOCATED_HOST_DIR ${CMAKE_CURRENT_LIST_DIR})
 
 set(CMAKE_SYSTEM_NAME Linux)
+set(CMAKE_SYSTEM_PROCESSOR @@CMAKE_SYSTEM_PROCESSOR@@)
 
 set(CMAKE_C_FLAGS "@@TARGET_CFLAGS@@ ${CMAKE_C_FLAGS}" CACHE STRING "Buildroot CFLAGS")
 set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS")
--
Quoted 1 line

1.8.4.5

Anyway, this looks good, so:

Regards,

Signature · 1 line
Samuel
#2 lore raw

PATCH v2 1/1 pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR.

In reply to Volker Krause

Dear Volker Krause,

package/pkg-cmake.mk host-cmake-package = $(call inner-cmake-package,host-$(pkgname),$(call UPPERCASE · line 191

Quoted 9 lines · Volker Krause
package/pkg-cmake.mk +9 -0
+# CMAKE_SYSTEM_PROCESSOR should match uname -m
+ifeq ($(BR2_ARM_CPU_ARMV5),y)
+CMAKE_SYSTEM_PROCESSOR = armv5
+endif
+ifeq ($(BR2_ARM_CPU_ARMV6),y)
+CMAKE_SYSTEM_PROCESSOR = armv6l
+endif
+ifeq ($(BR2_ARM_CPU_ARMV7A),y)
+CMAKE_SYSTEM_PROCESSOR = armv7l

I believe armv6l and armv7l is only valid on little-endian ARM systems, not big endian ones.

package/pkg-cmake.mk host-cmake-package = $(call inner-cmake-package,host-$(pkgname),$(call UPPERCASE · line 195

Quoted 4 lines
package/pkg-cmake.mk +4 -0
+endif
+ifndef CMAKE_SYSTEM_PROCESSOR
+CMAKE_SYSTEM_PROCESSOR = $(BR2_ARCH)
+endif

Are we sure that $(BR2_ARCH) is valid for all other cases?

Also, please use the following construct instead:

ifeq (....)
...
else ifeq (....)
...
else ifeq (....)
...
else
....
endif

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#3 lore raw

PATCH v2 1/1 pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR.

Quoted 13 lines · On Sunday 16 November 2014 23:23:42 Thomas Petazzoni wrote:

package/pkg-cmake.mk host-cmake-package = $(call inner-cmake-package,host-$(pkgname),$(call UPPERCASE · line 191

Quoted 9 lines · Volker Krause
package/pkg-cmake.mk +9 -0
+# CMAKE_SYSTEM_PROCESSOR should match uname -m
+ifeq ($(BR2_ARM_CPU_ARMV5),y)
+CMAKE_SYSTEM_PROCESSOR = armv5
+endif
+ifeq ($(BR2_ARM_CPU_ARMV6),y)
+CMAKE_SYSTEM_PROCESSOR = armv6l
+endif
+ifeq ($(BR2_ARM_CPU_ARMV7A),y)
+CMAKE_SYSTEM_PROCESSOR = armv7l

I believe armv6l and armv7l is only valid on little-endian ARM systems, not big endian ones.

you are right, and armv5 is missing the endianess suffix. Fixed in the next version.

Quoted 6 lines

package/pkg-cmake.mk host-cmake-package = $(call inner-cmake-package,host-$(pkgname),$(call UPPERCASE · line 195

Quoted 4 lines
package/pkg-cmake.mk +4 -0
+endif
+ifndef CMAKE_SYSTEM_PROCESSOR
+CMAKE_SYSTEM_PROCESSOR = $(BR2_ARCH)
+endif

Are we sure that $(BR2_ARCH) is valid for all other cases?

It is correct for x86 (32 and 64 bit), as discussed previously. Besides that I only have access to armv6 and armv7a devices for testing.

I looked at the kernel code, but unfortunately it's not exactly straightforward to determine what the machine name will be in which case. So, no, I can't provide a complete $(BR2_ARCH) -> uname -m mapping for all platforms.

That's of course sub-optimal, but IMHO not necessarily a blocker. This value is kinda unreliable from a CMake POV anyway (as it's provided by the system, especially when also considering non-Linux), and is therefore only useful for a very basic distinction between a fixed set of known architectures, if no better check is available, by means of some fuzzy comparison (see e.g. the x86 vs. i686 discussion earlier). You'd not use this for details like endianess or pointer sizes for example, there's more robust ways to check for that (e.g. CMAKE_SIZEOF_VOID_P). But an "is MIPS?" check would work no matter if the exact value might be "mips", "mipsel", "mips64" or "mips64l".

I therefore think that $(BR2_ARCH) is a sufficient approximation for all platforms where we do not know the exact mapping to uname -m.

Impact on existing packages is also worth looking at. There is very few actually making use of CMAKE_SYSTEM_PROCESSOR to begin with. I think it's safe to assume the case that a package breaks if the variable is set but worked with an empty one before is reasonably unlikely. So the ones making use of this are currently doing their own mapping and set CMAKE_SYSTEM_PROCESSOR via the command line (which is where the first version of the mapping for this patch came from). Those should not see a difference at all, they'd still be overriding it manually.

And anyone targeting a platform with a currently possibly wrong mapping with a new/updated package would need to sort that out in any case, while at least some platforms would work out of the box already with this patch.

So, IMHO still a step into the right direction, even if we cannot complete the $(BR2_ARCH) -> uname -m mapping entirely.

Quoted 11 lines

Also, please use the following construct instead:

ifeq (....)
...
else ifeq (....)
...
else ifeq (....)
...
else
....
endif

fixed

regards,
Volker

#4 lore raw

PATCH v2 1/1 pkg-cmake.mk: Set CMAKE_SYSTEM_PROCESSOR.

Hello,

Quoted 5 lines · Volker Krause
Quoted 2 lines

I believe armv6l and armv7l is only valid on little-endian ARM systems, not big endian ones.

you are right, and armv5 is missing the endianess suffix. Fixed in the next version.

Ok, thanks.

Quoted 32 lines
Quoted 1 line

Are we sure that $(BR2_ARCH) is valid for all other cases?

It is correct for x86 (32 and 64 bit), as discussed previously. Besides that I only have access to armv6 and armv7a devices for testing.

I looked at the kernel code, but unfortunately it's not exactly straightforward to determine what the machine name will be in which case. So, no, I can't provide a complete $(BR2_ARCH) -> uname -m mapping for all platforms.

That's of course sub-optimal, but IMHO not necessarily a blocker. This value is kinda unreliable from a CMake POV anyway (as it's provided by the system, especially when also considering non-Linux), and is therefore only useful for a very basic distinction between a fixed set of known architectures, if no better check is available, by means of some fuzzy comparison (see e.g. the x86 vs. i686 discussion earlier). You'd not use this for details like endianess or pointer sizes for example, there's more robust ways to check for that (e.g. CMAKE_SIZEOF_VOID_P). But an "is MIPS?" check would work no matter if the exact value might be "mips", "mipsel", "mips64" or "mips64l".

I therefore think that $(BR2_ARCH) is a sufficient approximation for all platforms where we do not know the exact mapping to uname -m.

Impact on existing packages is also worth looking at. There is very few actually making use of CMAKE_SYSTEM_PROCESSOR to begin with. I think it's safe to assume the case that a package breaks if the variable is set but worked with an empty one before is reasonably unlikely. So the ones making use of this are currently doing their own mapping and set CMAKE_SYSTEM_PROCESSOR via the command line (which is where the first version of the mapping for this patch came from). Those should not see a difference at all, they'd still be overriding it manually.

Quoted 6 lines

And anyone targeting a platform with a currently possibly wrong mapping with a new/updated package would need to sort that out in any case, while at least some platforms would work out of the box already with this patch.

So, IMHO still a step into the right direction, even if we cannot complete the $(BR2_ARCH) -> uname -m mapping entirely.

Yes, I agree. I know that being sure that all $(BR2_ARCH) values are valid is difficult, and we probably should simply rely on autobuilders
+ user testing to find out whether it causes some problems.

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com