User Details
- User Since
- Apr 15 2014, 12:19 PM (429 w, 1 d)
Sun, Jul 3
Sat, Jun 18
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.
Apr 7 2022
It's a pity this one didn't make it into 14.0.0, as you now can't even include <ranges> without immediately getting an error:
Mar 27 2022
Mar 15 2022
Mar 14 2022
Mar 1 2022
Feb 21 2022
I can't vouch for any of the PowerPC specific stuff, but I would like to get it into FreeBSD if at all possible. :)
Feb 12 2022
Feb 11 2022
Feb 9 2022
FWIW, this breaks compilation of Firefox:
Feb 8 2022
LGTM (@emaste I've never used the standalone build, always as part of the 'full monty' all llvm components)
Hm @pkubaj do you remember if this is needed for 32-bit PowerPC on FreeBSD? I can't remember...
Feb 7 2022
In any case, LGTM for this review itself of course.
Having said that, if this document is supposed to be published on GitHub, and read by people primarily via GitHub, I suppose it would make sense to change all the mentioned usernames to GitHub usernames instead of Phabricator usernames?
Note: andykaylor is Andy's GitHub username: https://github.com/andykaylor
and similarly, smaslov-intel is Sergey's: https://github.com/smaslov-intel
As far as I know:
Andy Kaylor = @andrew.w.kaylor
Sergey Maslov = @smaslov
Feb 5 2022
Address review comments.
Feb 4 2022
Jan 27 2022
Jan 25 2022
The creduce run is almost finished, I'll go create a ticket now.
The top 10 list from perf report output shows that it's doing quite a lot of StringMapImpl work:
Jan 24 2022
For some reason, this commit causes a huge memory usage, and extreme slowdown, when compiling the FreeBSD port math/openturns (https://github.com/openturns/openturns) port; see https://bugs.freebsd.org/261341.
Jan 21 2022
Jan 20 2022
How do you mean "no practical difference"? _VSTD is defined as std::_LIBCPP_ABI_NAMESPACE, and in turn, _LIBCPP_ABI_NAMESPACE is configurable by the user, or defaults to mangling the ABI version in it. So this would be a huge change, for unknown benefit?
Jan 16 2022
LGTM, thanks!
Jan 11 2022
LGTM too. I will commit this now.
Jan 3 2022
Dec 23 2021
Okay, in my case the test case is quite a bit shorter:
David, in this particular change, does "valid and handled successfully" mean "not crash or assert"? I just ran into the following assertion when building part of llvm (13.0.0) itself under FreeBSD 12:
Dec 14 2021
Dec 13 2021
It's a pity this never landed. Now libc++ can't work on older arm without non-standard patches... :)
Dec 10 2021
FWIW, this commit turned out to break the FreeBSD dns/bind916 port, see https://bugs.freebsd.org/259921.
Dec 9 2021
Dec 8 2021
This breaks building on FreeBSD, where I see:
Dec 6 2021
From a fellow FreeBSD user I received a test case which *appears* to be fixed by this particular review/commit, and which otherwise leads to an assertion:
Dec 4 2021
Dec 3 2021
Dec 2 2021
Nov 28 2021
Address review comments.
Nov 25 2021
FWIW, this fixes both the minimized test case from PR52594, and the full original test case from https://bugs.freebsd.org/260000.
Nov 24 2021
Nov 23 2021
Squash commits because arc tricked me.
Drop trailing dots from report messages.
Nov 22 2021
LGTM
Note that this is the version as we have it in the FreeBSD base system version of llvm 13.0.0. Without the isPPC64() check, we would get unexpected R_X86_64_REX_GOTP relocations in our kernel modules, which the kernel module loader cannot handle; see e.g. https://reviews.llvm.org/D109090#3113552 and further comments.