User Details
- User Since
- Jul 22 2016, 1:13 PM (246 w, 5 d)
Today
Use __builtin_va_copy instead.
Yesterday
Fix failed test case from pre-commit CI.
Mon, Apr 12
Fri, Apr 9
Thu, Apr 8
Update diff to trigger pre-CI rebuild.
Wed, Apr 7
LGTM.
Mon, Apr 5
Thanks for working on this version of the patch. It's always good to have something concrete to talk about instead of imaging how it's going to look like when everything is in place.
I'm still not convinced that dynamically allocating csect groups is the way to go though.
As you could see in the patch, the code are more complicated with dynamic allocation, and less efficient(doing more loopings to find the correct CsectGroup).
Also, we now lose control of how the CsectGroup are going to layout inside of a section, and how each section is going to layout in the object file (It now comes in a first come first serve order I believe).
This could bring some practical issues for us. For example, we may prefer to have layouts like: text -> data -> bss -> tdata -> tbss -> dwarf raw datas. But now, I think it's possible that we start having text -> data -> tdata -> bss -> tbss -> dwarf raw datas, which is not quite right when you have tdata cut in between of normal datas.
And I don't see a lot of benefit of having a single vector to contain DWARF sections and csect section. In D97184, we actually handles DWARF sections and csect sections quite differently (i.e. a large block of if/else statement for dwarf and csects).
I would still prefer only dynamically allocating DWARF raw datas, and have pre-defined csect sections. It's less of an issue to dynamically allocating DWARF raw datas because we do not really care the layout between different dwarf raw datas (as long as they are at the end of the object files).
Thu, Apr 1
Remove extra conditions because we only intended to target the headers for clang compilers.
@ldionne Hi Louis, what do you think about this patch? Do you have any ABI implication concern?
Fri, Mar 26
Making basic_filebuf's __open consistent with other __open.
Wed, Mar 24
@hubert.reinterpretcast Does the freelocale approach look good to you?
Use freelocale instead to avoid using thread local on Z.
Tue, Mar 23
Add back setlocale and use __ prefix in case of user macro expansion.
Use thread_local for newlocale variable.
Use "C" locale when NULL is passed.
Mon, Mar 22
Got it. Make sense. I removed the test case in case we want to have the declaration be more like what standard have in the text.
Ping for reviews from AIX's perspective. @xingxue @daltenty @hubert.reinterpretcast @Xiangling_L
Fri, Mar 19
Wed, Mar 17
Are all those people whose names I don't recognize AIX people? Any of them want to chime in with a "yes, this looks appropriate for our target"?
Rebase so that buildbot could build.
@Quuxplusone While waiting for the second reviewer, I added an extra change since you approved: do not build cxa_thread_atexit.cpp on AIX.
Could you take a quick look to see if that's okay?
Added do not build cxa_thread_atexit.cpp for libcxxabi on AIX.
Tue, Mar 16
FYI, I decide to review this patch before D97049 because I wouldn't know if the NFC make sense without the context of this patch.
Mar 15 2021
I think what we usually do for test case is to upstream some pre-compiled binaries for testing purpose. But that's not the way community preferred anyway because those binaries are baked, and might be hard to reproduce later. Given this patch is small and pretty self-explanatory, I'm okay with skipping the test and let D97184 to "test" it.
FYI @hubert.reinterpretcast in case he think we should still upstream a pre-compiled binary for testing.
Otherwise, the patch LGTM.
Thanks @Quuxplusone for the review.
Ping for a second reviewer in libc++ review group.
Address Arthur's comment.
Mar 12 2021
Gentle ping.
Mar 4 2021
LGTM if the nit comment is addressed
There are two items that we need to follow up after this patch:
- The corner case/bug in https://reviews.llvm.org/D95518#inline-919030
- The possibility of addressing the FIXME for https://reviews.llvm.org/D95518#inline-918884
Mar 3 2021
Mar 2 2021
Mar 1 2021
Feb 26 2021
Feb 9 2021
LGTM with minor nit.
Jan 12 2021
LGTM with minor nits.
Jan 11 2021
Jan 6 2021
Dec 22 2020
Dec 10 2020
Thanks. LGTM.
Please allow some time for @hubert.reinterpretcast to go over again before you land.
Dec 9 2020
Rebased on the latest parent.
ping.
Dec 8 2020
Address comments.
Dec 7 2020
Dec 4 2020
Dec 3 2020
Dec 2 2020
Manually closing this because I spelled the Differential Revision wrong.
Address comments and rebase.
Dec 1 2020
Address comments.
Address comments.