User Details
- User Since
- Aug 19 2016, 10:21 AM (301 w, 1 d)
Thu, May 26
Wed, May 25
Rebase
Define macro to 1
Address review comments
This breaks cross-compilation, because we're building pseudo-gen for the target but then trying to run it on the build machine. You'll need to do something like TableGen does (see https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/CrossCompile.cmake and the uses of build_native_tool). That being said, I'm trying to figure out why our builds are attempting to build this in the first place, since as far as I can tell nothing external depends on it yet.
Tue, May 24
Better MSAN suppression
XFAIL with MSAN, similar to other tests
Use sizeof instead of raw number
D126343 implements the process_vm_readv approach
Mon, May 23
LGTM, thanks. I'm surprised these weren't caught by the lint running on the diffs where they were introduced.
Fri, May 20
Thu, May 19
Tue, May 17
In the LLVM Windows toolchain, we work around this by creating a directory of symlinks: https://github.com/llvm/llvm-project/blob/8527f32f0a16a77edb0b53a5ab8074e518eeff54/llvm/cmake/platforms/WinMsvc.cmake#L133-L157. A VFS overlay would be a nicer solution, and I'm excited to have this supported. I'll leave the review to @rnk and @mstorsjo though.
Removing from my queue in the meantime
I think this might potentially break cross-compilation on Windows. We're in a NOT WIN32 block here, but that means you're not targeting Windows; you could still be targeting e.g. Linux but building on a Windows machine. I think NUL is the Windows equivalent, and you could have a CMAKE_HOST_WIN32 block, but it'll depend on how CMake invokes an execute_process command. (I have a Windows machine handy and can experiment with that, if you'd like.) That being said
We're seeing an internal failure after this change as well :( Great catch, but it looks like it exposed some existing brokenness.
Mon, May 16
Fri, May 6
LGTM
Planning changes to remove from people's queues while I address the comments (which will take a bit, since I'm out next week). Thanks for the reviews!
Thu, May 5
Tue, May 3
LGTM after changing to an error with a warning downgrade. Resigning to clear my queue, plus the Apple folks should get the final accept here :)
Mon, May 2
IIUC this only affects IDE project generation. This makes sense to me, though I'll give others a few days to take a look as well in case they have any thoughts on the organization.
Thu, Apr 28
Commandeering to abandon, since this landed elsewhere
Clearing my queue.
Is this still relevant? Requesting changes to clear my queue.
Sorry, this fell off my radar! Your reasoning makes sense to me, and in particular, I'd forgotten that only compiler-t has the Darwin multi-arch setup (and you'd still need to handle libc++/libc++abi/libunwind differently).
Requesting changes since there are unaddressed reviewer questions.
While we're figuring this out ... @ldionne, @phosek, do the runtimes build changes (https://libcxx.llvm.org/BuildingLibcxx.html) also mean that we assume a full LLVM checkout is present? If so, we could move this header to LLVM and have both libunwind and LLD use it from there, which would sidestep the issue here.
(clearing my queue while there are unaddressed comments)
I'm not really sure about the ".md being more standard than .txt" in the summary, but I guess it's more standard with Github (where Markdown gets nice rendering), so this makes sense.
Apr 27 2022
If I check out this commit and then check out the previous commit, clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp and clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected become modified in my working directory; their line endings are changed from CRLF to LF. That seems undesirable.
The following files have their line endings (when checked out on disk) changed from CRLF to LF by this patch. Seems harmless, but I just wanted to confirm that it was expected:
Adjust tvos simulator architecture detection as well
Apr 26 2022
Address review comments
I don't think many people are gonna notice a warning in the general spew of CMake configuration, tbh. Would it be too disruptive to make this an error instead and have a CMake option you can pass to get rid of the error?
Apr 25 2022
Apr 22 2022
To be clear, this is to support a build scenario where you only have the source tree under lld available?