[Bug]: Build fails with -Werror on 32-bit platform
#25
Assignees
Labels
bug
Something isn't working
Comments
|
Also, it needs to link to |
|
Thanks very much for the report. The timed_task move issue is fixed in mainline. The other two issues may depend on platform and toolchain. Are these only PPC, ARM, MIPS 32-bit issues? Unfortunately I don't currently have any machines set up to replicate. Would you be willing to try updating the moodycamel concurrentqueue library locally to see if that fixes the first problem? It may be that this is something that just popped up with newer versions of GCC (I've personally only tested up to GCC 11), and they may already have fixed it in moodycamel. If that is the case, I'll be happy to update the copy. The second problem likely needs a CMakeLists.txt update to pull in libatomic. The trouble is that I don't have a clear sense of when that is required. Maybe if CMake can detect a 32-bit build we could always link. |
|
@graphicsMan Thank you for responding. For atomics, something to this effect should work: uxlfoundation/oneTBB#987 I did not try other GCC versions, but it is easily doable, I have at least 6, 7, 10, 11 and 12 to test. |
|
Besides, I got some errors when trying to build with tests enabled, perhaps some compatibility issues, but did not dig into that yet. Will update on that once done. Without tests I got the build working, though on 32-bit I had to patch out -Werror. On 64-bit it builds as is. |
|
Wonderful. We currently do the vast majority of our work on 64-bit, and even with 32-bit, internal to the company we are not using CMake. Any way you can help is welcome. I'd love PRs or patches, but I'll try to get to these either way (biggest concern is setting up the correct cross-compiling toolchains, etc... which I don't have that much experience with). |
|
One PR is there #24 though it is orthogonal to issues discussed in this ticket. |
|
Thanks very much @barracuda156 . Sorry about the long time lag on response here; January is a tough month for quick responses. I'm attempting to get a cross-compile setup going so I can try building for Linux ARM32 (though I'm looking back and seeing a lot of Mac stuff in your ticket, would this need to be done on Mac?). Sorry, I'm pretty new to cross compilation. I have a Linux system (x86_64) CentOS 8 Stream with distrobox able to run e.g. Debian 12. I also have Mac M1 (and also a windows box, which is unlikely to be helpful). If you have suggestions for how to most quickly replicate your findings using the resources I have, I'd appreciate it. |
|
@graphicsMan Thank you for responding. While I have no setup to try building anything on Linux or arm32 in general, I think it should be a valid test-case for |
|
Okay, interesting . I just managed to successfully build and link libdispenso.so with x-compile arm32 on Linux. Now I'm able to repro the missing libatomic stuff. I've looked at various projects (TBB, taskflow, folly), and none of these seem to support arm32 or ppc32. I'm unsure the right way to add libatomic only as required, though I have been searching around and found this approach, which may or may not be sufficient: https://stackoverflow.com/questions/69281559/cmake-library-order-in-cmake-required-libraries-to-test-a-minimal-program-while Do you have experience fixing this (with CMake in particular) for your projects? |
|
@graphicsMan There are PRs both to Yes, neither of them supports 32-bit platforms from their side and they are super-slow and/or reluctant in reviewing/merging PRs (perhaps due to no commercial interest), but I have about 90% of For |
|
Thanks for the reminder. I'll see if I can use one of those as a basis for a fix over the next couple days
Get Outlook for Android<https://aka.ms/AAb9ysg>
…________________________________
From: Sergey Fedorov ***@***.***>
Sent: Wednesday, January 31, 2024 5:31:27 PM
To: facebookincubator/dispenso ***@***.***>
Cc: Brian Budge ***@***.***>; Mention ***@***.***>
Subject: Re: [facebookincubator/dispenso] [Bug]: Build fails with `-Werror` on 32-bit platform (Issue #25)
@graphicsMan There are PRs both to folly and oneTBB to fix libatomic linking: facebook/folly#2126 The second one I refer above. Yes, neither of them supports 32-bit platforms from their side and they are super-slow and/or reluctant in reviewing/merging
ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
ZjQcmQRYFpfptBannerEnd
@graphicsMan<https://github.com/graphicsMan> There are PRs both to folly and oneTBB to fix libatomic linking:
facebook/folly#2126<facebook/folly#2126>
The second one I refer above<#25 (comment)>.
Yes, neither of them supports 32-bit platforms from their side and they are super-slow and/or reluctant in reviewing/merging PRs (perhaps due to no commercial interest), but I have about 90% of folly tests passing<facebook/folly#2128 (comment)> on ppc (slightly less than on arm64, 91% there), and oneTBB also works on ppc AFAICT (I did not try tests there for quite a while, there were some failures, I think larger part still passed).
—
Reply to this email directly, view it on GitHub<#25 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AJIRY7IUEUKHYRZJDJCAVI3YRLV67AVCNFSM6AAAAABBXVNSN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRQGMYTCMJQHA>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
@graphicsMan I have looked through earlier issues, maybe this can be borrowed? In Macports we can add needed flags conditionally via portfile code, that is how we fixed it for P. S. And thank you by the way for reproducing it on Linux! |
|
dispenso_atomic.PATCH |
|
@graphicsMan Thank you, I will test it tonight and let you know. |
|
@graphicsMan Yes, it works for me. Configure detects that without lib no full atomics: And flag is passed: |
|
Thanks @barracuda156 for reporting the issues, for the commit, and for testing my patch. I think there was one more issue you reported, related to compiler warning-as-error in moodycamel lib. I didn't repro that with my arm32 cross-compile. It may be that we want to treat moodycamel as a "system" library, so that we don't get warnings from that code. I'm not sure how to do that in CMake. I found this, but haven't had a chance to take a deeper look, and it might be a week or two until I can get back to this: https://stackoverflow.com/questions/52135983/cmake-target-link-libraries-include-as-system-to-suppress-compiler-warnings If you wanted to try something and verify it for your platform, I'd happily accept another commit, otherwise, I can try to get back to this in a bit. |
|
@graphicsMan Are those warnings in reality harmless (i.e. triggered by w/e, but do not imply issues with the code) or possibly not? Disabling |
|
Those warnings show up on |
|
@barracuda156 Yes, it is my understanding that this should be an invalid (though understandable) warning. It is my understanding that this will not be null in this scenario (we should have valid producer token at this point). This error is with GCC? I'd prefer not to turn off -Werror altogether, but it would be good to disable this instance of this warning. I'll try to see if I can find out how to do that with this (since the error itself is showing up in atomic_base.h). If you have ideas, I'd love to hear them |
|
@barracuda156 Can you please verify for me whether the attached patch will solve your issue? |
@graphicsMan Thank you! I am away from the testing hardware atm, gonna be back by middle of next week. Will update on this as soon as I can. |
|
Hey @barracuda156 just checking in to see if you had a chance to test the patch? |
|
@graphicsMan Will do tonight, thank you for a reminder. |
|
Dropping my patch to remove |
|
Bummer. Okay, I will have to look deeper, and/or possibly just try to disable this warning. Thanks for checking. |
|
@barracuda156 can you please try to add If one of these values works, please let me know which, and I will put something together that applies this when GCC is the compiler. I'd like to try 2 first, as it seems right to keep the warnings as strict as possible without being broken by them. Thanks! |
|
Building from 4ebef42 – that still fails: Same with |
|
Great info, thanks @barracuda156. I'll try turning that off for GCC. I hate doing it, as I usually want all diagnostics, but it really doesn't make sense in this case. I'll try to get something landed this week, and after that, I'll cut a new release that incorporates all of these small recent fixes. |
|
Can you please double check that this patch works for you? I've verified that I can build with g++ and clang locally on x86_64 with this patch. Thanks! |
|
@barracuda156 Just checking in. I fully expect this to work, but where I don't have an easy setup to test, I'm hoping you can help me verify. Thanks! |
|
@barracuda156 I went ahead and landed this change. Please try from main when you get a chance? I plan to cut a new release soon, but I'd love to have your confirmation that this fixes your issue |
|
Sorry for a delay, will do it tonight. |
|
Yes, from ac845f0 it builds fine with no patches or manual flag passing: |
|
@graphicsMan It also builds fine with tests enabled, though there are a number of errors when running tests: #29 |
|
I'm going to close this issue since we have now a narrower one against PPC32-bit test failures specifically in #29 |
|
@graphicsMan Sure |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Contact Details
No response
What happened?
Build fails on 32-bit platform when warnings are treated as errors.
Perhaps these are nothing critical, but if they are easily fixable, it may be worth addressing this.
Version
latest (Edge)
Code of Conduct
The text was updated successfully, but these errors were encountered: