unix: Remove usages of the MICROPY_FORCE_32BIT flag.#18667
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #18667 +/- ##
=======================================
Coverage 98.47% 98.47%
=======================================
Files 176 176
Lines 22845 22845
=======================================
Hits 22497 22497
Misses 348 348 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Code size report: |
|
Why can't we remove it on Windows as well? |
All the supported build methods mentioned in If somebody adds support for MinGW/Cygwin/MSYS2 Arm64 builds, then this option stops being relevant and it should be retired. Until then, it's doing what it's supposed to, so why removing it? :) |
|
This looks like a very sensible thing to do, to generalize x86 (32-bit) support to a cross-compilation target. That cleans things up nicely. But from a practical point of view I wonder why we need to install yet another toolchain when |
|
As far as I know, an x86_64 toolchain is not guaranteed to also compile code for x86 since it can be built to target a limited subset of CPUs (ie. not having Another possibility is to replace A separate i686 toolchain is also just one step towards having host-neutral CI environments, which would have been a follow-up PR. |
This commit forces usage of an i686 cross-compiler for 32-bits x86 Unix
port builds, instead of relying on the "-m32" flag passed to the
currently installed GCC version.
Before this change it was not possible to build an x86 native module on
anything but an x86/x64 machine, since the scripts used the generic
"gcc" compiler installed on the system the code was built on. Recent
Ubuntu versions (at least five years old now) provide a 32-bits x86
cross-compiler on all supported machines ("gcc-i686-linux-gnu" and
"g++-i686-linux-gnu"), and since the CI uses Ubuntu as its base OS, that
compiler is now used for x86 builds.
Installing said cross-compiler, though, conflicts with the
"gcc-multilib" and "g++-multilib" packages that are installed on the CI
image as part of the 32-bits jobs setup procedure. This means that all
32-bits x86 builds have to be migrated to the new cross-compiler as
well.
Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
This commit removes support for forced 32-bits builds of the Unix port, as it only worked on x64 machines and x86 usage is low enough these days to allow making changes on how the latter target is built. Passing such flag to the Unix makefile will stop the build, as the flag itself will be removed from MicroPython entirely (right now only the MinGW Windows builds use it) in subsequent commits. CI build scripts had to be updated to manually pass the necessary flags that were once implied by MICROPY_FORCE_32BIT, for all targets that are meant to build 32-bits binaries. Finally, documentation for the Unix port has been updated with a section explaining how to make x86 builds on x64 hosts work again. Given that the 32-bits builds now use a cross-compiler, then other machines for which such a compiler is available can build x86 binaries too (eg. AArch64 or RISC-V 64). Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
This commit moves the definition of the "MICROPY_FORCE_32BIT" build flag away from the core's Makefile definition, migrating it to the Windows port's Makefile instead. Both Windows and Unix ports used this flag, which was intrinsically x64 specific. Since the Unix port is no longer using it and the Windows port is currently limited to x86/x64 builds it makes more sense to move that flag's support away from MicroPython's core. This assures that MinGW builds will still operate as usual without any changes. Support for this flag will probably be removed from the Windows port (and therefore from MicroPython) when Windows/Arm builds will show up. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
|
I've rebased the PR to remove conflicts, just in case. |
|
I've tried to get this PR working on Arch Linux. First I tried building the That's not very interesting, and I really don't know where to start trying to fix that. So I moved on and thought to use clang instead, which comes already with support for many targets. That worked well: Then running it looks good: Also, all tests pass (using just Anyway, I do really like this PR, and I think that's acceptable in the case you want to use clang instead of gcc to cross compile it. But maybe those instructions for clang and the |
|
Glad you got it to work in the end. Updating the documentation to also mention clang is a more than reasonable request and in fact I've done so in my unpublished branch. However, natmods won't build with it :| The compiler is hardcoded to expect a GCC-style toolchain triplet cross-compilation string and expects to use GCC as a compiler [1]. That can be made it work with Clang and a similar command line set with [2], but then it complains about symbol redefinitions [3]. If you're OK with it, I'll tackle the [1] $ make CC=clang CFLAGS_EXTRA='--target=i686-unknown-linux-gnu' LDFLAGS_EXTRA='--target=i686-unknown-linux-gnu' ARCH=x86 -C examples/natmod/features0 V=1
make: Entering directory '/micropython/examples/natmod/features0'
CC features0.c
i686-linux-gnu-gcc -I. -I../../.. -std=c99 -Os -Wall -Werror -DNDEBUG -DNO_QSTR -DMICROPY_ENABLE_DYNRUNTIME -DMP_CONFIGFILE='<build/features0.config.h>' -fpic -fno-common -U_FORTIFY_SOURCE -fno-stack-protector -DMICROPY_FLOAT_IMPL=MICROPY_FLOAT_IMPL_DOUBLE --target=i686-unknown-linux-gnu -o build/features0.o -c features0.c
i686-linux-gnu-gcc: error: unrecognized command-line option ‘--target=i686-unknown-linux-gnu’
make: *** [../../../py/dynruntime.mk:227: build/features0.o] Error 1
make: Leaving directory '/micropython/examples/natmod/features0'[2] diff --git i/py/dynruntime.mk w/py/dynruntime.mk
index a8233fdfe..39e1187fd 100644
--- i/py/dynruntime.mk
+++ w/py/dynruntime.mk
@@ -2,6 +2,7 @@
# MPY_DIR must be set to the top of the MicroPython source tree
BUILD ?= build
+CC ?= gcc
ECHO = @echo
RM = /bin/rm
@@ -47,7 +48,7 @@ CLEAN_EXTRA += $(MOD).mpy .mpy_ld_cache
ifeq ($(ARCH),x86)
# x86
-CROSS = i686-linux-gnu-
+CROSS ?= i686-linux-gnu-
CFLAGS_ARCH += -fno-stack-protector
MICROPY_FLOAT_IMPL ?= double
@@ -110,7 +111,7 @@ CFLAGS_ARCH += -march=rv32imac -mabi=ilp32 -mno-relax
# bare metal RISC-V toolchain with Picolibc rather than Newlib, and the default
# is "nosys" so a value must be provided. To avoid having per-distro
# workarounds, always select Picolibc if available.
-PICOLIBC_SPECS := $(shell $(CROSS)gcc --print-file-name=picolibc.specs)
+PICOLIBC_SPECS := $(shell $(CROSS)$(CC) --print-file-name=picolibc.specs)
ifneq ($(PICOLIBC_SPECS),picolibc.specs)
CFLAGS_ARCH += -specs=$(PICOLIBC_SPECS)
USE_PICOLIBC := 1
@@ -129,7 +130,7 @@ CFLAGS_ARCH += -march=rv64imac -mabi=lp64 -mno-relax
# bare metal RISC-V toolchain with Picolibc rather than Newlib, and the default
# is "nosys" so a value must be provided. To avoid having per-distro
# workarounds, always select Picolibc if available.
-PICOLIBC_SPECS := $(shell $(CROSS)gcc --print-file-name=picolibc.specs)
+PICOLIBC_SPECS := $(shell $(CROSS)$(CC) --print-file-name=picolibc.specs)
ifneq ($(PICOLIBC_SPECS),picolibc.specs)
CFLAGS_ARCH += -specs=$(PICOLIBC_SPECS)
USE_PICOLIBC := 1
@@ -143,7 +144,7 @@ else
$(error architecture '$(ARCH)' not supported)
endif
-ifneq ($(findstring -musl,$(shell $(CROSS)gcc -dumpmachine)),)
+ifneq ($(findstring -musl,$(shell $(CROSS)$(CC) -dumpmachine)),)
USE_MUSL := 1
endif
@@ -175,8 +176,8 @@ LIBM_NAME := libc.a
else
LIBM_NAME := libm.a
endif
-LIBGCC_PATH := $(realpath $(shell $(CROSS)gcc $(CFLAGS) --print-libgcc-file-name))
-LIBM_PATH := $(realpath $(shell $(CROSS)gcc $(CFLAGS) --print-file-name=$(LIBM_NAME)))
+LIBGCC_PATH := $(realpath $(shell $(CROSS)$(CC) $(CFLAGS) --print-libgcc-file-name))
+LIBM_PATH := $(realpath $(shell $(CROSS)$(CC) $(CFLAGS) --print-file-name=$(LIBM_NAME)))
ifeq ($(USE_PICOLIBC),1)
ifeq ($(LIBM_PATH),)
# The CROSS toolchain prefix usually ends with a dash, but that may not be
@@ -224,12 +225,12 @@ $(CONFIG_H): $(SRC)
# Build .o from .c source files
$(BUILD)/%.o: %.c $(CONFIG_H) Makefile
$(ECHO) "CC $<"
- $(Q)$(CROSS)gcc $(CFLAGS) -o $@ -c $<
+ $(Q)$(CROSS)$(CC) $(CFLAGS) -o $@ -c $<
# Build .o from .S source files
$(BUILD)/%.o: %.S $(CONFIG_H) Makefile
$(ECHO) "AS $<"
- $(Q)$(CROSS)gcc $(CFLAGS) -o $@ -c $<
+ $(Q)$(CROSS)$(CC) $(CFLAGS) -o $@ -c $<
# Build .mpy from .py source files
$(BUILD)/%.mpy: %.py[3] $ CROSS= make CC=clang CFLAGS_EXTRA='--target=i686-unknown-linux-gnu' LDFLAGS_EXTRA='--target=i686-unknown-linux-gnu' ARCH=x86 -C examples/natmod/features0 V=1
make: Entering directory '/micropython/examples/natmod/features0'
CC features0.c
clang -I. -I../../.. -std=c99 -Os -Wall -Werror -DNDEBUG -DNO_QSTR -DMICROPY_ENABLE_DYNRUNTIME -DMP_CONFIGFILE='<build/features0.config.h>' -fpic -fno-common -U_FORTIFY_SOURCE -fno-stack-protector -DMICROPY_FLOAT_IMPL=MICROPY_FLOAT_IMPL_DOUBLE --target=i686-unknown-linux-gnu -o build/features0.o -c features0.c
In file included from features0.c:8:
In file included from ../../../py/dynruntime.h:40:
In file included from ../../../py/binary.h:30:
In file included from ../../../py/obj.h:31:
../../../py/mpconfig.h:195:18: error: redefinition of typedef 'mp_int_t' is a C11 feature [-Werror,-Wtypedef-redefinition]
195 | typedef intptr_t mp_int_t;
| ^
./build/features0.config.h:3:18: note: previous definition is here
3 | typedef intptr_t mp_int_t;
| ^
In file included from features0.c:8:
In file included from ../../../py/dynruntime.h:40:
In file included from ../../../py/binary.h:30:
In file included from ../../../py/obj.h:31:
../../../py/mpconfig.h:196:19: error: redefinition of typedef 'mp_uint_t' is a C11 feature [-Werror,-Wtypedef-redefinition]
196 | typedef uintptr_t mp_uint_t;
| ^
./build/features0.config.h:2:19: note: previous definition is here
2 | typedef uintptr_t mp_uint_t;
| ^
2 errors generated.
make: *** [../../../py/dynruntime.mk:228: build/features0.o] Error 1
make: Leaving directory '/micropython/examples/natmod/features0' |
Great, thanks! It might also be worth documenting the obvious fallback for x86-64 gcc toolchains: I tested that with this PR and it works as expected, and the tests pass.
Yes, that's a good idea, thank you. |
|
#19308 should, in theory, add Clang support for natmods. It is far from being complete, but that should keep me busy for a little while. Once sorted out it'll be interesting to see the size/speed difference of the two compilers for Arm and RV32/RV64. Hopefully one day we'll see https://github.com/espressif/llvm-project/ upstreamed so Xtensa could join in on the fun too. |
Summary
This PR removes usage of the
MICROPY_FORCE_32BITbuild flag for the Unix port, and forces cross-compilation for x86 targets in doing so.MICROPY_FORCE_32BIThas always been x64-specific and right now there is support for one more mixed 32/64-bits architecture (RISC-V) on which such flag would make builds fail. Since x86 usage has declined over the years, this PR also treats x86 as a cross-compilation target, for which a separate toolchain is needed.Now x86 builds on non-x86 hosts require having a
CROSS_COMPILEvariable passed to the makefile to force building an x86 binary. This applies to native modules as well, so an i686 cross-compilation toolchain is now needed to build x86 native modules. This also allows building x86 native modules on non-x86/x64 hosts, as long as the proper toolchain is installed. Documentation for the Unix port has updated accordingly to show how to create x86 builds with these changes.CI jobs had to be updated to force usage of the new cross-compiler for 32-bits builds. This means that on Ubuntu the
gcc-i686-linux-gnuandg++-i686-linux-gnupackages need to be installed instead ofgcc-multilibandg++-multilib.MICROPY_FORCE_32BIThasn't been completely removed from MicroPython, though. It is also used by the Windows port to create MinGW builds, and since it is the last remaining target needing such a thing and being limited to x86/x64 targets anyway it is not much of a deal to limit the flag support to that specific port.This is one less roadblock in having the Unix port CI test suite run on AArch64-based runners (the only build-related issue is forcing usage of
x86_64-linux-gnu-as the compiler for x64 native modules and on non-x64 hosts for an x64 QEMU-based target).Testing
Besides manual builds, this was successfully built and tested by CI on my own branch.