User Details
- User Since
- Apr 15 2014, 12:19 PM (466 w, 4 d)
Fri, Mar 24
Sun, Mar 19
LGTM
Sat, Mar 18
Uploaded wrong commit.
Simplify.
Thu, Mar 16
Mon, Mar 13
Is the -srcdir option no longer needed?
This was already fixed by @aaronpuchert in rGc1a31ee65b3a2bf2b452febb99703b3fdfa43328, in a slightly different way, but functionally equivalent. Test cases were also updated.
Sun, Mar 12
Tue, Mar 7
Yes, this looks good to me. Tried with very recent main:
Mon, Mar 6
Sat, Mar 4
I think this is fine. @emaste can you think of any objections? 8 and 9 are long gone.
Fri, Mar 3
Note that the original problem was not only about _Float16 (COMPILER_RT_HAS_FLOAT16 in CMake), but also __bf16 (COMPILER_RT_HAS_BFLOAT16 in CMake). Both failed on i386-freebsd.
Mon, Feb 27
Sat, Feb 25
LGTM, are there no test cases which are affected by this?
Feb 21 2023
LGTM
Yes, this is quite a bit nicer. Maybe run the isPPCSecurePlt() function through clang-format, just to be sure of the style?
Feb 20 2023
Yeah, this looks quite a bit nicer, and should be more maintainable. Thanks.
Feb 18 2023
LGTM, some minor clang-format nits, but these aren't critical (to me at least :)
Jan 24 2023
Yes, this should save some time while compressing.
Jan 23 2023
Ugh, this is really a messy situation. COMPILER_RT_HAS_FLOAT16 (as a CMake feature) is also used in compiler-rt/test/builtins/CMakeLists.txt, where it is unconditionally used for each arch. I received some failures in buildbots, e.g.:
Jan 22 2023
Abandoning in favor of D136044.
Rebase onto main.
Btw these changes are independent of whether -m32 is used or not, they only check ${arch}. So if compiler-rt is ever changed to go over all support arches with e.g. -target x-y-z, this change would still work as-is. I'm mostly interested in getting the out-of-the-box build going on FreeBSD again, as it has been broken since 15.0.0.
Dec 17 2022
FWIW the BSDs never had lseek64; off_t was 64 bit from the original BSD 4.4 Lite sources... :)
Dec 15 2022
Dec 14 2022
Nov 9 2022
Oct 16 2022
Another note to clarify: the problem is that the builtins are built *twice*, once for x86_64 and once for i386, the latter using the -m32 compiler flag. In the CMake configuration phase however, the feature detection is always done with the default compiler flags, i.e. without -m32, and therefore it will find features that aren't supported for 32-bit targets.
@brooks btw didn't you encounter this problem during llvm-devel port builds ? E.g. it dies with:
Oct 4 2022
LGTM
Sep 14 2022
Yes, this should probably be fine for 16.
Aug 20 2022
I'm fine with it, adding @brooks so he's aware for the FreeBSD llvm port.
Aug 7 2022
So I've been trying again with the following diff applied on top of this one:
Aug 5 2022
Address review comments.
Aug 4 2022
Aug 3 2022
Unfortunately it doesn't really change anything on the FreeBSD build side. It still shows similar output when I put some messages in there:
GCC 12 -Wformat -pedantic emits a warning:
warning: ISO C17 does not support the ‘%b’ gnu_printf format [-Wformat=]The behavior is not ported (and it's unclear to me how to implement it).
LGTM.
Jul 30 2022
Okay, so do we continue with this approach at all, then? The problem that x86_64 builds fail by default on non-gcc compilers still stands. I'm just not good enough in handling the CMake logic to figure that out, if we have to handle it via CMake.
Jul 29 2022
Jul 28 2022
Fix typo.
Jul 19 2022
LGTM, are there any tests that need to be enabled explicitly?
Jul 12 2022
Jul 3 2022
Jun 18 2022
Jun 4 2022
To answer my own question partially, the original commit is Howard's: rG088e37c77aafaec5ead8fbe7ebf918265e6b86f2:
Despite my pathological distrust of spin locks, the number just don't lie. I've put a small spin in __sp_mut::lock() on std::mutex::try_lock(), which is testing quite well. In my experience, putting in a yield for every failed iteration is also a major performance booster. This change makes one of the performance tests I was using (a highly contended one) run about 20 times faster.
May 31 2022
In any case it would be nice if it was possible to decide whether ABIv2 will be pinned down (and ABIv3 is made into the 'new' unstable) before 15.0 is branched. :-)
May 26 2022
...
But for 14.0, which is our trunk/main/head, there is no such ABI promise, therefore we can simply bump libc++.so to .2 now and fix the issue that way. If the upstream libc++ ABI version gets bumped to 2 though, we'll have to bump our .so to .3 I'm afraid, since the ELF soversion doesn't really allow "half-sized" version numbers.
Do you have to bump the version now or would it be possible for you to wait maybe one or two months so we can get things sorted out upstream?
Thanks a lot. I'll make sure to put up a libc++ review now to get rid of the FreeBSD std::pair problem in the future! :)
May 23 2022
May 21 2022
...
There's a subtle difference between the two std::vector constructors, mentioned at https://en.cppreference.com/w/cpp/container/vector/vector, where variant 3 is "Constructs the container with count copies of elements with value value" (so it needs a copy constructor), and variant 4 is "Constructs the container with count default-inserted instances of T. No copies are made". E.g. the latter should not require a copy constructor, but just a default constructor.
May 17 2022
...
Ultimately, this is caused by _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR in /usr/include/c++/v1/__config (this was last modified by @EricWF, but a long time ago). If this setting is enabled (to preserve the old ABI), std::pair will always have a non-trivial copy constructor, and since IntervalMap contains a std::pair (via RootLeaf -> IntervalMapImpl::LeafNode -> NodeBase<std::pair<KeyT, KeyT>, ValT, N>), IntervalMap itself is also non-trivially copyable. std::vector in turn requires its value type to be copy-constructible (at least certainly when you use the constructor with a size).
I was hoping something I could reproduce without having FreeBSD myself - either on godbolt (though godbolt might not have different OS platforms as a primary feature) or something like that. Or a brief example that emulates the quirk of FreeBSD to better understand what we're working around here.
I can confirm that this fixes both my original full test case, and the minimized variant. Since this change resembles D78668, I think it is relatively safe to apply now, and merge to the release/14.x branch?
May 15 2022
May 12 2022
Cross-reference: https://github.com/llvm/llvm-project/issues/55414
Apr 29 2022
Add llvm/utils/gn/secondary/lldb/include/lldb/Version/BUILD.gn while we're here.
Apr 8 2022
Sure, LGTM.