Fixes for older macOS and atomic builtins detection#1907
Conversation
|
@Orvid has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Those Fedora checks failing in 0 sec cannot be due to the code changes, I guess. |
|
I will see why it fails now, allow me a bit of time. On Mac it should build in fact, since it builds on Macports buildbots. |
|
@Orvid Referring to your reply earlier: #1835 (comment) P. S. Non-Mac and non-PPC test result in checks should be irrelevant (I looked at some, and appears they are). I could not have broken anything on new MacOS too, I believe. |
| base64URLDecodeSWAR}; | ||
| } | ||
| #endif | ||
| #if defined(__POWERPC__) // PowerPC BE |
There was a problem hiding this comment.
Does this get defined for PPC64 LE, or only 32-bit PPC BE?
There was a problem hiding this comment.
Also would prefer to check for FOLLY_PPC if this is only ppc 32-bit be.
There was a problem hiding this comment.
@Orvid __POWERPC__ includes __ppc__ and __ppc64__ – only Big endian, and for practical purposes only Darwin. (I think it was also used on BeOS.)
I generally try to be specific, since I have no way to test on *BSD, AIX or Linux, which could also use ppc32. Perhaps FOLLY_PPC_BE may do, since we want to have macOS in one define, but do not include ppc64le (well, not in general case).
There was a problem hiding this comment.
Added a define for non-Apple PPC Big endian. Hope it is correct.
87f4e95 to
f4c49e1
Compare
|
@Orvid Please take a look. I updated the patched, taking into account your recommendations, and verified that |
|
@Orvid has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: RISC-V is a growing platform, and Folly and commonly used dependency. We want to make sure that any projects that relies on Folly can be ported to RISC-V. I'm verifying that everything builds and run correctly by cross-compiling it locally and passing the test suite using QEMU. Pull Request resolved: #2104 Reviewed By: yfeldblum Differential Revision: D51691588 Pulled By: Orvid fbshipit-source-id: 9c4ae6718ffbf6b55432bf6e1ffba06311c7d345
Summary: Some platforms need libatomic linked for sub-word operations while others need it for super-word operations. Let's make sure we test for all. Relates to #1907 Pull Request resolved: #2126 Reviewed By: Gownta Differential Revision: D52975209 Pulled By: yfeldblum fbshipit-source-id: 2c8781f817d6cc59fb8c74c1d13812e584187b47
Summary: Some platforms need libatomic linked for sub-word operations while others need it for super-word operations. Let's make sure we test for all. Relates to facebook/folly#1907 X-link: facebook/folly#2126 Reviewed By: Gownta Differential Revision: D52975209 Pulled By: yfeldblum fbshipit-source-id: 2c8781f817d6cc59fb8c74c1d13812e584187b47
With these fixes
follybuilds on older macOS, including 10.5.8 and 10.6 on PowerPC. I have put PPC-related ones into a separate commit.One non-macOS-specific fix is related to atomic builtins detection. Current
follyconfigure code does not work correctly: even when atomics requires linking tolibatomic, the test still passes, as if it is not needed:Performing Test FOLLY_CPP_ATOMIC_BUILTIN - Success; sure enough, the build fails at linking stage.I borrowed the check from here, where it has been tested on macOS and Linux 32-bit: uxlfoundation/oneTBB#987