- User Since
- Oct 21 2013, 6:34 AM (400 w, 22 h)
Mar 18 2021
It appears another subset of this might have been merged in https://reviews.llvm.org/D77767 also?
Feb 10 2021
Feb 8 2021
This looks like the bug I reported in https://bugs.llvm.org/show_bug.cgi?id=41629. Should I close it, or did you want to add that as a test?
On second thought, since the SHARED build is now fixed, there is no reason to disable it (and it helps eliminate more missing link problems).
Okay, the first problem is because add_llvm_library doesn't support STATIC, so we need to use llvm_add_library instead (though it is also fixed by adding the missing Support dependency). The second problem is also missing dependency (for CommonTableGen).
Reverted. Seems that something is going wrong with -DBUILD_SHARED_LIBS=ON with this.
LGTM, modulo comments from previous reviewer
Dec 11 2020
needed for memmove too
Dec 3 2020
I think with the move to newer C++ standards, some of the missing functionality in C could be replicated (or use the _WIN32 code paths for __CYGWIN__ in more places). I tried that for a bit, but it was pieces like struct sigaction, realpath, and getsid missing, which are very platform-specific.
Nov 30 2020
Nov 23 2020
Nov 16 2020
I think this, and similar recent commits, are causing the shared library builds to fail some tests if this code gets linked into libLLVM.so: https://bugs.llvm.org/show_bug.cgi?id=48181. I assume it might actually a bug in ld (GNU Binutils for Ubuntu 2.34), as I don't understand the linker behavior there?
Nov 14 2020
Thanks for identifying that. I think I can re-land just without that part of the test. It is not essential, I added it just to try to check all of the dwarf-related output.
Nov 12 2020
Nov 9 2020
Nov 6 2020
Ah, okay, yes that is more clearly needing to be fixed. I think I just need to rearrange how that command line option works.
add null test as requested, rebase
Nov 5 2020
If it's too fiddly I don't mind this going in without a test case right now -- I think extra cycles would be just as well spent bringing up / testing JITLink for ELF. We should make sure we don't replicate this issue by using empty strings as sentinels there.
The .cfi_startproc directive is part of the unwind information (eh_frame), and is not debug info, so that seems like a possible bugfix even? I'd expect that observation to be consistent with the goal of this PR, though I didn't see specifically where that pass was being affected in a cursory glance back through the PR.
Nov 3 2020
use helper method
I think these need to be handled in these places, given how clang emits relocations in .dwarf sections. For some contextual history, this was added as a sentinel for absolute symbols in https://github.com/llvm/llvm-project/commit/ad6d349fbcb2f9ced3c57b5b61231315175bcb47 (and later adjusted in https://github.com/llvm/llvm-project/commit/8f1f87c73487dcb83cf5d6d44b062f1e9796fae6). If I compare readelf --relocations of clang vs. gcc, I see GCC emit a name for these symbols for x86 inside the .dwarf sections, but that clang uses the name NULL, which gets translated there into the empty string.
Oct 16 2020
In my first attempt, I missed that it's intentional we're claiming MMI->hasDebugInfo() if there's information available for DWARF output if using fast-isel, even if we don't need that DebugInfo because the output for it is disabled (CodeView information is still somewhat separate, as my attempt to bring them back in line is what failed the CI testing). Fortunately, that's what tests are for when this change was added in 8763c0c5b7a566d2fe9476259aa346640b0a9deb (https://reviews.llvm.org/D53885). I think this should pass all tests now, as I've now fully removed that part of the change (it passes locally anyways).
Yep, I saw that too. I pushed a revert commit while I wait for the tests locally to rerun and confirm my fix for that.
@rnk I've generalized the API to your suggestion and added a test. Let me know what you think, thanks!
Oct 15 2020
Removed NFCI, since as I look back, I think this also enabled optimization of constant structures, and I was misusing the NFCI term
I've removed NFCI, since I ended up changing some of the SVE handling when attempting to rebase it
Oct 7 2020
Makes sense to me
Sep 22 2020
@rnk would you be an appropriate reviewer for this idea and willing to look it over? I see @timurrrr originally created this generalization that I'm proposing exposing (r196288 / 1cd1444449cb68e2091e8f36785403288ebd3326), but that he's not working with the llvm project anymore. Thanks!
fixing commit diff
rename helper functions to canBeUnquotedInDirective
Sep 3 2020
Sep 1 2020
I'm glad to see this is gone. I was just about to file a bug report that this code used to incur a deadlock risk when unw_step was used from a signal handler since macOS 10.9.0 on x86_64, since Apple had removed the other side of this code circa keymgr-28, so it now had been attempting to call malloc. (I'd observed this deadlock occur in testing, and implemented a work around for it downstream, in https://github.com/JuliaLang/julia/pull/37101/commits/fad04d39d592d8e0fcbfba439e8157f582bbc850#diff-86a37833b03dbe7f0874e30469cb1d46R93-R102)
Jul 20 2020
Does that mean I should commit it there also, or how does that process work?