-
Notifications
You must be signed in to change notification settings - Fork 867
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
Skip aligned allocation on SV39 MMUs #949
base: master
Are you sure you want to change the base?
Conversation
If the host has an SV39 MMU, the usual aligned hinting strategy will not work despite the system being 64-bit. On RISC-V linux systems, the type of MMU can be read from /proc/cpuinfo. If we find one, a constant is defined that will allow us to skip the aligned hinting when the time comes.
This constant will be defined when the host has an SV39 MMU. The usual 2TiB+ hinting strategy will not work on those systems, so we skip it, just as we would if the system was not "64-bit."
On 32-bit or SV39 systems, _mi_os_get_aligned_hint() in src/os.c is ifdef'd to always return NULL. As a result, the OS prim allocation routines using that hint will fail to obtain aligned memory. When that happens, we free() the result, and overallocate. But this is wasteful: we know that (with high probability) the first attempt at an aligned allocation will fail. This commit modifies mi_os_prim_alloc_aligned() to skip the doomed alloc and subsequent free using the same ifdef condition that is used to disable the hint.
@microsoft-github-policy-service agree |
I've tested this on my Milk-V Jupiter system (SpacemiT M1 SoC), and I'm no longer seeing the warnings, as expected. I'm wondering, tho, if this should be a runtime check rather than a build-time check. Are there other RISC-V MMUs where the >2TiB allocation would work? If so, this could cause an issue for distro packages which may be built on a machine with a different MMU but get run on a machine with sv39. Edit: This build-time check definitely won't work when cross-compiling. |
Hey, thanks for taking a look. I know basically nothing about cross-compiling with CMake, but with autotools, you're supposed to override the (computed) settings for the build system with the values for the host system. In this case that's still possible because the setting is passed to the compiler (or preprocessor) via The binary distro thing is more of an issue. As a highly enlightened user of a source-based distro, this had not occurred to me :P There are indeed other RISC-V MMU types where the hinting should be enabled, and distros will not be making special binaries targeted at each MMU. Runtime detection would be better in this scenario, but it also makes the value of this proposition a lot fuzzier. We're only avoiding an alloc/free with this detection -- it's not a huge savings. If we stop to parse |
For now I have integrated the cmake check directly (since the other |
Yep, fine by me. How wrong are the changes to |
An attempt at fixing #939
The SV39 detection works well enough with my sample size of 1. But I also had to add some extra
#if
s tomi_os_prim_alloc_aligned()
to avoid the pointless alloc/free. I don't have any 32-bit hardware any more, but this should speed things up there, too.