-
Notifications
You must be signed in to change notification settings - Fork 262
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
GraphBLAS: Add support for generating the JITpackage when cross-compiling #616
GraphBLAS: Add support for generating the JITpackage when cross-compiling #616
Conversation
Do you have a branch with zipped source? Otherwise I'll have to try to modify our build system to work with a branch. Should be publicly accessible though. I'm away from work now so I could try in 12 hours or so. |
Maybe here: https://codeload.github.com/mmuetzel/SuiteSparse/zip/refs/heads/graphblas You can also use GitHub's interface on the "Code" tab and click on the green "Code" button right above the list of files in the repository and click on "Download ZIP" there. That's how I got the above link... |
Okay, I checked out dev and applied the changes from this pull request. Unfortunately it fails with some strange messages:
|
Oof. That looks like an infinite recursion. Most likely a think-o in some of the conditions. Do you have access to the full build logs? Could you please share them? |
I have to see if I can do that because there might be proprietary stuff in the logs. Will have to ask for permission. Do you have a discreet way to send or upload that log? |
I've found a typo in the target name that might have caused this. It should have been I rebased on a current head of the I also added the option to pass additional arguments to the native CMake build (e.g., a toolchain file for the build platform). I don't actually know if that caused the infinite regression. If you are uncomfortable sharing the log here, you could also send it to me via email. You should find my email address as the author of a couple of commits in this repository. |
To my surprise, the beta7 branch built successfully with -DSUITEPARSE_USE_FORTRAN=OFF and replacing the find_package calls. Is there anything in this branch that replaces the changes above? |
The change that removed the generated file was reverted for that beta. But as @DrTimothyAldenDavis pointed out: That can only be a temporary change. So, we need a more permanent solution for cross-compiling. |
Given that we might want to upstream this, I renamed the new flag to I tested this in MXE Octave (cross-building from Linux to Windows). That seems to be working. |
There's no rush, however. I can keep the GB_JITpackage.c file in the final SuiteSparse 7.4.0 stable release. |
I thought this was going to be much more of a pain to implement. (It still was tricky. But a fun-project anyway.) There have been a lot of changes to the build system between 7.3 and now. There was already very useful feedback from packagers and the community. I very much appreciate that feedback. 👍 If something breaks horribly, we could probably follow up with a 7.4.1 release. (When it comes to this PR, reverting the removal of the JIT package file would be an easy work-around.) But we should still wait for @Desperado17's test results before merging this. |
Ok so I checked out dev2 branch and replaced the two files with the ones from the pull request. Recursion still there. |
Could you please show the log? Or at least the last 20 lines or so until the error? |
It's a recursion. It just keeps printing larger messages. How do I pinpoint the ciritcal part? |
Just the last 20 lines or so are ok. Similar to what you showed before... |
A few more. After these, I aborted manually. [ 4%] Built target mongoose_test_io |
Which command do you use to configure SuiteSparse? |
In theory only the -DSUITESPARSE_USE_FORTRAN=OFF FLAG. Plus the parameters the toolchain sets of course. |
Which parameters does the toolchain set? |
I can try sending an anonymized version of cmakecache.txt if that helps. |
Is there no way you can check how cmake was called for configuration? The cmakecache.txt might help. (But please make clear which parts you anonymized.) |
Could you please also show which build platform and which target platform you are using? |
Host is an Ubuntu 22.04 x64 . Compiler is a gcc-9.3.0-2.31 aarch 64 cross compiler by linaro. Anonymization is only the prefix of the file path of the source/build dir which has been replaced by /proprietary/. Target platform is an aarch64 based poky linux. |
Need to leave for two hours. Will read messages later. |
It looks like it never picks up a native compiler in your configuration. In that case, you'd need to explicitly let CMake know which compilers it should use to build a native binary.
You might need to adjust the path to a native C compiler for your build system. Save a file with that content, e.g., to Then configure with If that doesn't help yet, you might need to override more settings in the toolchain file (e.g., If you like, please upload that |
For comparison, here is the file that I find in the folder |
I removed the lines again and those cross flags did not reappear only your error message. Something seems wrong here. |
Please clarify: What did you remove from where and which flags did not reappear where? And what do you mean by "your error message"? |
With "your error message" I mean "Native toolchain not configured correctly" I removed the following lines again from the linux_host.toolchain And it still printed Native toolchain not configured correctly although I expected the error from to reappear. I had to delete the build directory to bring it back. Readding the flags yields again "Native toolchain not configured correctly". |
Could you please delete the current content of After that, upload the new |
Native toolchain not configured correctly |
What does |
Maybe try the following in # CMake settings for build host
set ( CMAKE_SYSTEM_NAME "Linux" )
set ( CMAKE_SYSTEM_PROCESSOR "x86_64" )
set ( CMAKE_C_COMPILER "gcc" )
set ( CMAKE_C_FLAGS "" CACHE STRING "" FORCE )
set ( CMAKE_EXE_LINKER_FLAGS "" CACHE STRING "" FORCE )
set ( CMAKE_MODULE_LINKER_FLAGS "" CACHE STRING "" FORCE )
set ( CMAKE_SHARED_LINKER_FLAGS "" CACHE STRING "" FORCE )
# do not look into package registries
SET(CMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY ON)
SET(CMAKE_FIND_PACKAGE_NO_SYSTEM_PACKAGE_REGISTRY ON) Then delete the |
As far as I can tell these CFLAGS are an environment variable our build system sets. |
Does CMake still use the environment variables when you |
Native toolchain not configured correctly |
Good. Those wrong flags are gone now. 👍 Could you please copy the exact error message that you see now? |
-- SuiteSparse CMAKE report for: SuiteSparse -- build type: MinSizeRel -- Configuring done (3.7s)
-- Build files have been written to: /proprietary/build/system/ext-suitesparse-dev/1/workspace
-- Configuring incomplete, errors occurred! |
Hmmm. Maybe try again after adding the following line to the toolchain file: set ( CMAKE_CROSSCOMPILING OFF ) |
It seems to proceed much further now. Will tell you if overall build is success. |
If compilation succeeds, could you please show the content of the native toolchain file that finally worked? That might be helpful for others that might want to cross-compile GraphBLAS. |
Success! This is the content of the final linux_host.toolchain: # CMake settings for build host set ( CMAKE_CROSSCOMPILING OFF ) |
I'll try and add some documentation with hints for cross-compilers. That'll have to wait until after the holidays though. |
Sounds great. Thanks for all your help with this, @mmuetzel and @Desperado17 . Shall I merge this in now? Or shall I wait until you've had a chance to add some instructions? Presumably in the GraphBLAS/JITpackage/README.txt file? It would be fine with me if the instructions were specific to an x86 linux host. Someone else who is cross compiling with another host could probably use those instructions to figure out a reasonable host toolchain of their own. I'd like to post a stable 7.4.0 sometime before the end of the year, if possible. |
Also, use mark-down syntax for formatting.
I've added some hints for cross-compiling to the README file, and changed its format to mark-down. |
Great! Is it ready to merge? I could do that and then post a new beta release. My goal for a stable 7.4.0 release is Dec 30. |
9c6ac8f
into
DrTimothyAldenDavis:dev2
I've merged it in. I haven't tested it in a cross compilation mode though. I will post another beta release first. If that looks good, with this change, I'd like to call it a stable 7.4.0, on Dec 30 if possible. Does that sound ok? |
That sounds ok. In case there are major issues, they could probably be fixed in a version 7.4.1... |
OK. I will release the stable SuiteSparse 7.4.0 tomorrow morning, my time. I can easily and quickly do a 7.4.1 if needed. |
Official CMake documentation suggests that CMake should be run multiple times when building a native binary is needed to generate files when cross-compiling. First: build native (including the native generator binary). Second: cross-build (using the native generator binary built in the first step)
But that is inconvenient and doesn't integrate well into SuiteSparse which consists of many subprojects.
This PR tries to implement an approach where calling cmake twice (once for the host and then again for the target) shouldn't be necessary. It still needs to configure twice: once before the native generator executable was built, and then again after it has been built as an external project. But it does that automatically.
I didn't actually test if this works. (I don't have a cross-toolchain here.) But I hope it does. Keeping this as a draft for now.
I'd very much appreciate if someone could test and report potential issues.
@Desperado17: Would you be up for that?