-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[powerpc] Some build errors and test results: issues to be addressed – threading, endianness #2128
Comments
Perhaps this:
Does not look great either:
|
Generally speaking, folly only supports 64 bit architectures. There may be bits and pieces that work on 32-bit platforms, but overall Folly is entirely untested on them. If the fixes needed to get it compiling on 32-bit PPC are reasonably small, we can probably accept a pull request. |
@Orvid I had the basic build fixed for |
@Orvid I got tests running on I hacked pthreads for now, just setting |
@Orvid @pkubaj
For the context, on macOS 14.2.1 arm64 I got 91% passing, 279 failures, and I had to remove one test since it was simply freezing. |
The log from tests is almost 7 GB, my PowerMac refuses to even open it Few quick notes before digging into it further:
Anything mentioning overflow is not surprising, since this is 32-bit arch. Tests using long double will likely fail since, I would believe, nothing accounts for IBM format of those. To be updated. |
PicoSpinLock.h: error: no match for call to '(const folly::make_atomic_ref_t) (folly::PicoSpinLock<long long int, 63>::UIntType&)'
@Orvid Unfortunately, most of the failures produce no meaningful output in logs, so to figure out what fails I will need perhaps to run those separately via GDB. However, I will summarize below meaningful failures. Gonna make it similar-problem-per-comment to make it readable, dropping repeated error messages. |
Also, there is no real monotonic clock on older macOS, and Macports provides a drop-in implementation which relies on what is actually available in the kernel. It is a compromise and cannot serve as a 100% substitute. This could be another reason.
Perhaps related as well:
|
|
|
Same story here:
|
|
There should be a way to use So this test failed as a result of my imperfect preliminary fix to the problem:
|
This is more of a note to myself, it has to be checked elsewhere too. |
|
crc32cSoftware and crc32cAutodetect all fail, but hardware ones pass. |
|
It is also used in Can we use |
Etc. |
|
@Orvid While there was a typo in the initial version of one patch, which was used with the test run above, I corrected it, and also dropped reverting a few commits related to thread caching, rebuilt and rerun tests, results are nearly identical:
(I disabled a couple of known-to-be-broken-tests, and there is one new for thread caching, which expectedly fails, but otherwise I do not see anything notable. Gonna check logs tomorrow, this already took some 20+ hrs in a row.) On the other hand, and ironically, on the latest macOS 14.3 arm64 tests are just marginally better with 91% passing vs 90% on 10.6 ppc :) |
Re time, perhaps we can use |
Summary: To support `constexpr` we just need to bypass `loadUnaligned()`. Reviewed By: ilvokhin Differential Revision: D57908121 fbshipit-source-id: 6409f4e0157478a0c711b376ef35af42d4150856
What happened here? |
They made nonsensical comments on multiple issues, so I removed the comments they made. |
@Orvid Thank you for clarifying that. By the way, do we have chances getting at least some of fixes merged? There is no need to rush with some experimental ones for BE archs, of course, but some are/should be uncontroversial. I can understand that as long as no one with an interest in BE platforms and some understanding of what to do passes by, issues are stalled, that’s fine. I need somebody from upstream willing to verify and merge proposed fixes (doing it selectively is fine). Since perhaps nobody will have time to test those on every existing macOS version, I suggest the following: as long as a patch does not break officially supported platforms and has no obvious potential to introduce regressions, rely on MacPorts buildbots results for x86 and my opinion for PowerPC. Without some degree of trust and good will this won’t proceed otherwise. |
Could someone help me a bit with an advice on the following? (I do not ask to fix it for me, but perhaps for someone who knows what the code does exactly, pointing out at the issue will be fairly easy.)
This is the error I get:
It seems that
folly/synchronization/AtomicRef.h
uses a kind of SFINAE, which does not work as intended either due to implicit assumption of 64-bit arch (so pointers and offsets are wrong) or due to incorrect assumption of bool and spinlock being equal to 1 byte (both are 4 byte onppc
).Any ideas and suggestions will be greatly appreciated.
The text was updated successfully, but these errors were encountered: