Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cross-compilation for arm-none-eabi target #121

Closed
danngreen opened this issue Jan 18, 2022 · 7 comments
Closed

Cross-compilation for arm-none-eabi target #121

danngreen opened this issue Jan 18, 2022 · 7 comments

Comments

@danngreen
Copy link

Hi, very nice project!

I've found that two changes need to be made in order to cross-compile for a "bare-metal" ARM target using arm-none-eabi-gcc (that is, the gcc arm embedded toolchain).

  1. In float_common.h, there is this block:

#if defined(__APPLE__) || defined(__FreeBSD__)
#include <machine/endian.h>
#elif defined(sun) || defined(__sun)
#include <sys/byteorder.h>
#else
#include <endian.h>
#endif

With every version of the toolchain from 10.3.1 going back to at least 5.4, endian.h is located in machine/endian.h, so line 47 needs to match a macro that for the ARM target. I've experimented and found both || defined (__arm__) and || defined (__ARM_EABI__) work, but I'm not sure which is proper (nor do I know where this file is located with propriety non-gcc-based ARM compilers).

  1. In digit_comparison.h:
    int32_t shift = -am.power2 + 1;
    cb(am, std::min(shift, 64));

The arm-none-eabi-g++ compiler complains no matching function for call to 'min(int32_t&, int)'. Explicitly casting the integer constant to the same type as the variable works, and seems to be stylistically similar to other casts in this file:

cb(am, std::min(shift, int32_t(64)));

If these changes seem helpful to others, I'm happy to issue a pull request. Or try different solutions if this breaks someone else's build. In any case, thanks for the great project!

@lemire
Copy link
Member

lemire commented Jan 18, 2022

@danngreen Please do submit a PR. It seems entirely reasonable.

@jwakely
Copy link
Contributor

jwakely commented Jan 18, 2022

See #122 and #123 which fix both these.

@jwakely
Copy link
Contributor

jwakely commented Jan 18, 2022

In #122 I fixed the min deduction with std::min<int32_t>(shift, 64) but I can change it to the cast if you prefer that.

For the header, it's newlib that provides the <machine/endian.h> header, not the CPU, so it would be better to detect __NEWLIB__ not just __arm__ or __ARM_EABI__. Detecting newlib would also fix it for risc-v, cris-elf, and countless other targets that don't match the __arm__ macro. But #123 has what I think is a better fix anyway.

@danngreen
Copy link
Author

Excellent, yes it did occur to me to do something like what you did in #123 (looking for the required symbols first), so I'm glad to see you did that. I can confirm this compiles with the arm-none-eabi. Thanks!

@jwakely
Copy link
Contributor

jwakely commented Jan 18, 2022

Those PRs have been merged, so I hope arm-none-eabi works now and this can be closed, @danngreen

@lemire
Copy link
Member

lemire commented Jan 18, 2022

Please verify.

@danngreen
Copy link
Author

Yes, thank you, it builds now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants