search

raspberrypi / userland

Source code for ARM side libraries for interfacing to Raspberry Pi GPU.
BSD 3-Clause "New" or "Revised" License
2.05k stars 1.09k forks source link

CMakeLists: Introduce AMD64 (to fix x86_64/--native builds) #670

Open flokli opened 3 years ago

flokli commented 3 years ago

CMakeLists.txt currently assume everything not ARM64 to be ARM. When compiling on x86_64-linux, this pulls in some binaries that are arm32 only.

Some tools make some sense to have accessible even on non-ARM workstations. For example, dtmerge can be used to merge .dtb files with overlays on a x86_64 machine (while mounting an sdcard for example).

Simply by adding this conditionals, we're able to get a dtmerge binary that runs on x86_64-linux.

6by9 commented 3 years ago

Have you actually tested this on a Pi?

ARM64 is a define passed in from the buildme script - https://github.com/raspberrypi/userland/blob/master/buildme#L24

I don't see you having added a similar flag for ARM, therefore it is never going to be defined and you'll never build any of the libraries under you if (ARM) conditionals.

MMAL should build for x86_64 - it was one of the test platforms for the framework, and used to be used under Windows. There may be some 32bit assumptions/checks in some of the structs. Those should be resolved when we unrevert the 64bit support. Yes the Khronos libraries are going to be borked where they include ARM assembly.

The ./buildme --native option does appear to be broken (if it has ever really worked). Fixing that to choose a sensible list of applications to build would be useful.

flokli commented 3 years ago

I wasn't using the ./buildme script - the build runs in some sandboxed environment without access to sudo (and outputs need to go to a custom location), so it won't work. I was essentially invoking cmake directly, and passed in ARM(64)=1 to it.

I updated a new version, which now includes the necessary changes in the ./buildme script. Also, changed ARM to be ARM32, to make things more clear this is only ON on the armv6/7 architectures.

MMAL should build for x86_64 - it was one of the test platforms for the framework, and used to be used under Windows. There may be some 32bit assumptions/checks in some of the structs. Those should be resolved when we unrevert the 64bit support.

Yes, this indeed didn't build for me. For now, I made it ARM32 only. Also, had to disable some targets that link against it.

I successfully built this natively for x86_64-linux and aarch64-linux, and cross-compiled from x86_64-linux to armv6l-linux and aarch64-linux. I couldn't test on armv6/7l natively (as I don't have access to the hardware).

6by9 commented 3 years ago

TBH I'd prefer to go the other way - default to ARM and add a NATIVE flag.

This repo is specifically targetted at the Pi, therefore that is the absolute priority, anything else is the odd-ball. There will also be forked repos that build via cmake and rely on the current behaviour, so minimising the behavioural changes to those is important.

flokli commented 3 years ago

How would the semantics of NATIVE be? No cross-compilation involved?

If we build this on a raspberry pi or on x86-64-linux, that's both no cross-compilation, it's just that less of the tools can build in the x86-64-linux case.

6by9 commented 3 years ago

i was following the `--native option from buildme (https://github.com/raspberrypi/userland/blob/master/buildme#L20). ARM/ARM64 has already been filtered off, so it's build for the host vs cross-compile on non-ARM platforms.

Perhaps better to add it as an X86 option to avoid confusion. At least it makes it explicit which platform it's being targetted at.

flokli commented 3 years ago

Yeah, X86 might be more understandable, but then, we'd need to distinguish between I686 and AMD64 (and every other host cpu we might build this for).

It's really a pity CMake has no standardized concept to get this information, like meson or autotools have.

I'll change this to AMD64, because X86 could also mean i686, alongside with some comments.

6by9 commented 3 years ago

If you'd make the --native option of buildme set the new flag it'd be appreciated.

flokli commented 3 years ago

I updated the patch, now introducing the AMD64 flag (and some documentation in /CMakeLists.txt on the reasoning.

Successfully natively compiled on x86_64, aarch64, and crosscompiled from x86_64 to armv6,7,aarch64.

mweinelt commented 3 years ago

We are actively relying on this patch downstream in NixOS and we'de be happy if it could be merged upstream.