User Details
- User Since
- Apr 4 2019, 2:54 PM (163 w, 3 d)
Fri, May 20
LGTM, thanks!
Wed, May 18
LGTM, thanks!
Wed, May 11
Resolved by https://reviews.llvm.org/D125141
Fri, May 6
Differentiate between inlines hidden and regular visibility in test
Any guidance as to how I should proceed?
Thu, Apr 28
Make assert for mixed non-export visibility and dllexport a fatal error
Wed, Apr 27
LGTM, we haven't shipped any of the C++17 and up interfaces on AIX yet, so this seem fine to me.
Tue, Apr 26
@daltenty - what do you want to do about it? Add LIBCXX_ENABLE_WERROR=OFF in the AIX build config (which also would affect the 64 bit build), do silence it differently somehow?
@ldionne It's worth noting what's in this patch right now won't be our final ABI lists unfortunately. Our build compiler currently doesn't implement visibility support, so _LIBCPP_HIDE_FROM_ABI and friends don't have the intended effect. We're actively working on visibility for LLVM 15, but until that is all in we won't have a build compiler that can generate a stable ABI.
Mon, Apr 25
This change seems ok in principal, but these functions are extern 'C", so now that they're no longer internal linkage they'll provide the same symbol as the real thing. Adding such functions with linkable POSIX reserved names into user program seems like we are asking for trouble. Can we get these moved into some kind of namespace or at least with some kind of double underscore prefix?
Make MCWasmStreamer ignore MCSA_Exported
Apr 22 2022
Update the patch based on feedback on the RFC (https://discourse.llvm.org/t/rfc-adding-exported-visibility-style-to-the-ir-to-model-xcoff-exported-visibility/61853/8). There seem to be limited visibility bits, and the implementation has kind of reserved space for internal, so adding exported may cause problems. On the other hand dllexport appears to match the semantic and currently only be used as a marker to indicate to some passes that we should preserve the symbol since it will be exported from shared objects (which is exactly what we want too), so I've updated the implementation as suggested to use dllexport instead.
Apr 21 2022
It's something unique to AIX that we add shared objects to archives for our usual shared library implementation. I'm guessing with this implementation of the patch, we'll end up dumping all symbols together regardless of what archive member they belong to?
LGTM, thanks!
Apr 20 2022
Thanks for getting this fixed this so quickly and sorry for the messy revert.
Apr 19 2022
Address review comments
Apr 18 2022
Mar 23 2022
LGTM as an interm fix. We'll revert and replace this once we have a fix in Google Benchmark.
Right. But that's not the question i asked, i believe?
Mar 15 2022
LGTM, thanks!
Mar 7 2022
LGTM, similarly to the other platforms indicated the bits can be set through the filesystem interface (though our chmod -h is a no-op)
We could pass the linker an export list with the desired visibility for the functions in test on AIX if we need the visibility for the test. This is likely what we'll have to do in other places in libc++ until clang on AIX has real visibility support.
Rename remaining ABI lists
Mar 4 2022
Rebase to trigger CI
I trust that you're right when you say it affects the ABI, but do you have an example (I'm sure there's many, I just can't think of one right now).
Gentle ping
Mar 1 2022
Rebase to retrigger CI
Gentle ping
Feb 25 2022
clang-format
clang-format
Feb 24 2022
Feb 17 2022
LGTM, just copying the longer rational why the message is expected on AIX.
Feb 16 2022
LGTM, thanks!
Add back in CmdArgs.push_back(Args.MakeArgString(Plugin)) that got accidentally removed
Feb 15 2022
- Rebase
- incorporate suggestion from review
- add fuse-ld=ld to debugger tuning test, to make it invariant when running on platforms that might use ldd by default
clang-format
Add missing semicolon
Feb 14 2022
Change seperator option type to bool
Feb 8 2022
Add additional context
Feb 7 2022
Update to a new version that requires the compiler to define the __LIBC_NO_CPP_MATH_OVERLOADS__ macro (which clang does).
Remove whitespace change
Run clang-format
Feb 6 2022
Feb 1 2022
Slightly tweak whitespace formatting
Fix missing context on diff
Jan 31 2022
Confirming LGTM, thanks!
LGTM with minor nit. We'll see how we can better remediate some on on AIX these later, but looking at the existing CMake, we're not the only ones skipping some of these.
LGTM, with minor nit
Jan 29 2022
Thanks for pinging us on this. After taking a look at the AIX test failure, and dumping the error_code we get back from the new implementation, I think this is actually due to some ambiguity in the expected errno when the combination of O_DIRECTORY and O_NOFOLLOW is used and the path is a symlink.