Page MenuHomePhabricator

smeenai (Shoaib Meenai)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 19 2016, 10:21 AM (217 w, 3 d)

Recent Activity

Yesterday

smeenai added a comment to D89639: [lld-macho] Emit empty string as first entry of string table.

Do we have any logic to emit an n_strx for an empty string? If so, that would need to be adjusted to emit 1 instead of 0. If not, LGTM.

Mon, Oct 19, 7:03 PM · Restricted Project
smeenai added a comment to D89177: [cmake] Add support for multiple distributions.

Ping. Assuming everyone is okay with the direction, any comments on the change itself?

Mon, Oct 19, 2:24 PM · Restricted Project, Restricted Project, Restricted Project

Fri, Oct 16

smeenai accepted D89555: [CMake] Don't add -lstdc++ to LDFLAGS.

Both CMake and the driver (assuming it's invoked as clang++/g++) should take care of adding the C++ standard library, so this LGTM.

Fri, Oct 16, 11:43 AM

Thu, Oct 15

smeenai added a comment to D89177: [cmake] Add support for multiple distributions.

That isn't what I meant. It's entirely okay for the runtimes to be driven via AddExternalProject like the runtimes build does, since that's akin to having a separate CMake invocation for each configuration. That's okay.

What I'm saying is that if the next logical step is to also add support for multiple distributions in libc++'s build itself (e.g. adding LIBCXX_<DISTRIBUTION>_ENABLE_SHARED & al), then I don't think that's a good idea.

Totally agree. That would be the path compiler-rt’s Darwin build goes, and it is a frequent problem.

Thu, Oct 15, 5:10 PM · Restricted Project, Restricted Project, Restricted Project

Wed, Oct 14

smeenai added inline comments to D88922: [CMake] Avoid accidental C++ standard library dependency in sanitizers.
Wed, Oct 14, 11:55 PM · Restricted Project
smeenai accepted D88922: [CMake] Avoid accidental C++ standard library dependency in sanitizers.

This is needed to avoid potential race conditions where some of the C++ headers have been copied but not all of them, and some other runtime that doesn't depend on the C++ headers sees this incomplete state and gets a spurious build failure, so LGTM.

Wed, Oct 14, 6:15 PM · Restricted Project
smeenai updated subscribers of D89177: [cmake] Add support for multiple distributions.

We've already considered introducing a similar mechanism so thank you for working on this! There's one issue that I haven't figured out how to resolve and I'd be interested in your thoughts: building executables and libraries in the most optimal may require incompatible flags. For example, when building a shared library you have to use -fPIC but for executables that's suboptimal; similarly, when building executables you may want to use LTO but if you also want to create a distribution that contains static libraries, that's a problem because those will contain bitcode. So far our solution has been to simply do multiple builds with different flags (in GN this can be done in a single build by leveraging the "toolchain" concept that also allows sharing as much work as possible, but I haven't figured out how to do anything like that in CMake).

Good question. We need something similar as well, where some clients of the libraries want them built with RTTI (because their programs rely on RTTI and you can get link errors when linking them against non-RTTI LLVM libraries), but we don't wanna build the executables with RTTI because of the size increases.

We recently ran into this as well where we need RTTI for LLVM libraries but don't want to enable it for the toolchain.

I don't know of any way to do this in CMake other than just configuring multiple times with the different flags, like you said. Maybe we could implement some logic for that in the build system, but I'm not sure if it's worth it. (I'm wondering if the Ninja multi-config generator added in CMake 3.17 could be useful for this, although I haven't played around with that at all yet.) I do recognize it limits the usefulness of this approach quite a bit (since if you're doing separate configurations you can just do different LLVM_DISTRIBUTION_COMPONENT settings), but I'm hoping it can still be valuable for simpler scenarios.

One idea I had would be to eliminate the use of global variables like CMAKE_C_FLAGS and CMAKE_CXX_FLAGS and always set these via target_compile_options (which is something we should do in any case to move towards more modern CMake); since we always use functions like add_llvm_component_library to create targets in LLVM, we could then have these create multiple targets that would differ in flags like -fPIC or -frtti and set up dependencies among these appropriately. It means you'd have to build files multiple times (I don't think there's any way around that) but you'd save time on things like tablegen.

Wed, Oct 14, 3:40 PM · Restricted Project, Restricted Project, Restricted Project
smeenai updated the diff for D89177: [cmake] Add support for multiple distributions.

Address review comments

Wed, Oct 14, 3:29 PM · Restricted Project, Restricted Project, Restricted Project

Tue, Oct 13

smeenai added a comment to D89177: [cmake] Add support for multiple distributions.

Reading up on the Ninja multi-config generator a bit, you can define your own build types, so you should be able to create e.g. one build type for LTO and another for PIC. You'd still need some external logic to define that you use the LTO mode for executables and the PIC mode for libraries, but the multiple distribution support should work nicely in conjunction with that.

Tue, Oct 13, 12:07 AM · Restricted Project, Restricted Project, Restricted Project

Mon, Oct 12

smeenai added a comment to D89177: [cmake] Add support for multiple distributions.

We've already considered introducing a similar mechanism so thank you for working on this! There's one issue that I haven't figured out how to resolve and I'd be interested in your thoughts: building executables and libraries in the most optimal may require incompatible flags. For example, when building a shared library you have to use -fPIC but for executables that's suboptimal; similarly, when building executables you may want to use LTO but if you also want to create a distribution that contains static libraries, that's a problem because those will contain bitcode. So far our solution has been to simply do multiple builds with different flags (in GN this can be done in a single build by leveraging the "toolchain" concept that also allows sharing as much work as possible, but I haven't figured out how to do anything like that in CMake).

Mon, Oct 12, 11:49 PM · Restricted Project, Restricted Project, Restricted Project
smeenai accepted D88468: [llvm-readobj] Don't print out section names for STABS symbols.

LGTM.

Mon, Oct 12, 3:11 PM · Restricted Project

Fri, Oct 9

smeenai updated subscribers of D89177: [cmake] Add support for multiple distributions.
Fri, Oct 9, 7:38 PM · Restricted Project, Restricted Project, Restricted Project
smeenai requested review of D89177: [cmake] Add support for multiple distributions.
Fri, Oct 9, 7:33 PM · Restricted Project, Restricted Project, Restricted Project
smeenai added a comment to D85140: [RFC] Factor out repetitive cmake patterns for llvm-style projects.

Also a styling nit, this should have been cmake/Modules, not cmake/modules. The former is used only by LLVM and is a historic relic, all other projects use cmake/Modules which is more common in CMake-based builds.

Fri, Oct 9, 3:35 PM · Restricted Project, Restricted Project
smeenai added a reviewer for D89142: llvmbuildectomy: beanz.
Fri, Oct 9, 11:25 AM · Restricted Project

Tue, Oct 6

smeenai added a comment to D88922: [CMake] Avoid accidental C++ standard library dependency in sanitizers.

LGTM, but I'll give the sanitizer folks a bit of time to review as well.

Tue, Oct 6, 1:50 PM · Restricted Project

Mon, Oct 5

smeenai added inline comments to D88176: Explicitly specify CMAKE_AR in WinMsvc.cmake.
Mon, Oct 5, 1:20 PM · Restricted Project
smeenai added a comment to D88843: [libcxx] Don't treat Windows specially with visibility annotations.

I think the reason for this restriction is that on Windows, __declspec(dllimport) changes the compiler's code generation to assume that the symbol will be imported (i.e. if the compiler sees a call to a function X which is marked __declspec(dllimport), it'll generate an indirect call to __imp_X instead). If you're linking against libc++ statically, the imported version of the symbol will of course not exist, so you want to disable the __declspec(dllimport) annotations by default if you're building just the static library for Windows.

Mon, Oct 5, 11:57 AM · Restricted Project

Fri, Oct 2

smeenai added a comment to D88629: [lld-macho] Add ARM64 target arch.

It is not entirely clear whether I use the "ARM64" (Apple) or "AArch64" (non-Apple) naming convention. Guidance is appreciated.

Fri, Oct 2, 7:13 PM · Restricted Project
smeenai added a comment to D88400: [llvm-objcopy][MachO] Add support for universal binaries.

LGTM barring the comment about more tests.

Fri, Oct 2, 6:43 PM · Restricted Project
smeenai accepted D88372: [Object][MachO] Refactor MachOUniversalWriter.

LGTM

Fri, Oct 2, 6:15 PM · Restricted Project
smeenai accepted D88756: [CMake] Don't use CMakePushCheckState.

LGTM

Fri, Oct 2, 2:37 PM · Restricted Project

Thu, Oct 1

smeenai added a comment to D88454: [CMake] Use -isystem flag to access libc++ headers.

@smeenai do you already have an alternative for your use case?

Thu, Oct 1, 11:49 AM · Restricted Project, Restricted Project
smeenai committed rGdcb5b6dfbfb5: [runtimes] Remove TOOLCHAIN_TOOLS specialization (authored by smeenai).
[runtimes] Remove TOOLCHAIN_TOOLS specialization
Thu, Oct 1, 9:54 AM
smeenai closed D88627: [runtimes] Remove TOOLCHAIN_TOOLS specialization.
Thu, Oct 1, 9:53 AM · Restricted Project
smeenai retitled D88627: [runtimes] Remove TOOLCHAIN_TOOLS specialization from Revert "[AIX] Try to not use LLVM tools while building runtimes" to [runtimes] Remove TOOLCHAIN_TOOLS specialization.
Thu, Oct 1, 8:50 AM · Restricted Project
smeenai added a comment to D88627: [runtimes] Remove TOOLCHAIN_TOOLS specialization.

This actually isn't exclusively a revert of D85329 "[AIX] Try to not use LLVM tools while building runtimes" as the title seems to suggest. Before that patch there were already explicit TOOLCHAIN_TOOLS specified in the call to llvm_ExternalProject_Add in the runtimes build, which already suppressed the target-specific tool selection as mention in the description here.

That said, I'm not opposed to the resulting change, which moves all that logic to one place rather than having an extra layer on top just for the runtimes build. We should clarify the title and description though, because removing the TOOLCHAIN_TOOLS here entirely will actually be a new change to how runtimes are built.

Thu, Oct 1, 8:49 AM · Restricted Project

Wed, Sep 30

smeenai requested review of D88627: [runtimes] Remove TOOLCHAIN_TOOLS specialization.
Wed, Sep 30, 6:05 PM · Restricted Project

Mon, Sep 28

smeenai added a comment to D88454: [CMake] Use -isystem flag to access libc++ headers.

I was abusing this functionality to be able to get the C++ headers installed without having to build all the runtimes build prerequisites; we have a configuration where we only need the C++ headers installed (and not the library), and using the runtime-libcxx-headers was an easy way to get them without needing to first build Clang and all the other tools the runtimes build requires. I should be able to come up with something else for that use case though.

Mon, Sep 28, 5:36 PM · Restricted Project, Restricted Project

Wed, Sep 23

smeenai updated subscribers of D87199: [lld-macho] Implement support for PIC.

but would any of this be easier if SyntheticSections were InputSections instead of OutputSections

Maybe... we could make them InputSections and then generate unsigned relocations for each of their entries. There would be some additional overhead (e.g. the addend field would be useless) but it would allow us to handle the rebases in one place. One thing I'm not sure about is whether other architectures have an equivalent of X86_64_RELOC_UNSIGNED that can be used for this; it looks like arm64 does, but not sure about PPC (not that it really matters).

There are other benefits to having SyntheticSections be InputSections: there would no longer be a need for the SectionPointerUnion, and DSOHandle could just be an ordinary Defined symbol.

I actually can't think of many real downsides... do you have any in mind? That said, the current architecture is not terrible, and I'm inclined to work on implementing more features and pushing the refactoring till later.

Wed, Sep 23, 11:15 PM · Restricted Project
smeenai added inline comments to D87199: [lld-macho] Implement support for PIC.
Wed, Sep 23, 10:58 PM · Restricted Project
smeenai committed rGee7ee71f40e9: Explicitly specify CMAKE_AR in WinMsvc.cmake (authored by Gwen Mittertreiner <gwenm@fb.com>).
Explicitly specify CMAKE_AR in WinMsvc.cmake
Wed, Sep 23, 6:06 PM
smeenai closed D88176: Explicitly specify CMAKE_AR in WinMsvc.cmake.
Wed, Sep 23, 6:06 PM · Restricted Project
smeenai added a comment to D88176: Explicitly specify CMAKE_AR in WinMsvc.cmake.

The LLVMExternalProjectUtils change is needed either way, but I believe the WinMsvc case to be a CMake bug, and I've put up https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5264 to fix it. (We should still go ahead with it as a workaround for now though.)

Wed, Sep 23, 5:12 PM · Restricted Project
smeenai accepted D88176: Explicitly specify CMAKE_AR in WinMsvc.cmake.

https://gitlab.kitware.com/cmake/cmake/-/commit/55196a1440e26917d40e6a7a3eb8d9fb323fa657 is the relevant CMake change, and I believe we need to add logic to find llvm-lib. I'll play with that.

Wed, Sep 23, 3:47 PM · Restricted Project
smeenai accepted D88176: Explicitly specify CMAKE_AR in WinMsvc.cmake.

It seems strange for CMake to prefer "ar" to "lib" when targeting Windows. Do you happen to know the CMake change that's responsible?

Wed, Sep 23, 11:55 AM · Restricted Project
smeenai accepted D88160: [lld-macho] cleanup unimplemented-option warnings.

You have a small typo in the commit message: s/ldd/lld/

Wed, Sep 23, 11:11 AM · Restricted Project

Tue, Sep 22

smeenai added inline comments to D87852: [lld-macho] Allow the entry symbol to be dynamically bound.
Tue, Sep 22, 2:24 PM · Restricted Project
smeenai accepted D87852: [lld-macho] Allow the entry symbol to be dynamically bound.

LGTM. It's interesting that this is a thing.

Tue, Sep 22, 12:52 PM · Restricted Project
smeenai accepted D87909: [lld-macho] Support absolute symbols.

LGTM

Tue, Sep 22, 12:21 PM · Restricted Project
smeenai accepted D87856: [lld-macho] Support -bundle.

LGTM

Tue, Sep 22, 12:06 PM · Restricted Project
smeenai accepted D87959: [lld-macho][NFC] Refactor syslibroot / library path lookup.

LGTM

Tue, Sep 22, 12:00 PM · Restricted Project
smeenai requested changes to D87960: [lld-macho] Always include custom syslibroot when running tests.

The test cleanups are really nice, but I'm not a fan of coding this sort of test-specific behavior (especially the path) into LLD itself :/ (I know we have some precedent with LLD_IN_TEST, but that's only used in canExitEarly and it's a pretty minor behavior change.)

Tue, Sep 22, 11:58 AM · Restricted Project

Mon, Sep 21

smeenai accepted D88069: [CMake] Use find_dependency in LLVMConfig.cmake.

LGTM

Mon, Sep 21, 11:02 PM · Restricted Project
smeenai accepted D88068: [CMake] Use append for CMAKE_REQUIRED_* variables.

LGTM

Mon, Sep 21, 11:00 PM · Restricted Project
smeenai added inline comments to D87780: [lld-macho] Export trie addresses should be relative to the image base.
Mon, Sep 21, 11:55 AM · Restricted Project

Sep 17 2020

smeenai added inline comments to D87780: [lld-macho] Export trie addresses should be relative to the image base.
Sep 17 2020, 5:00 PM · Restricted Project
smeenai accepted D87780: [lld-macho] Export trie addresses should be relative to the image base.

LGTM

Sep 17 2020, 3:58 PM · Restricted Project

Sep 16 2020

smeenai added a comment to D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early.

What was the conclusion for the comments about the priority level (100 vs. 101)?

Sep 16 2020, 1:09 PM · Restricted Project

Sep 9 2020

smeenai added a comment to D87199: [lld-macho] Implement support for PIC.

Is there a funnel point that we know that all the rebase entries will pass through to ensure that we catch them rather than adding them at a couple of sites?

It may be clearer if we scan the GOT/LazyPointerSection inside RebaseSection::finalizeContents() for any absolute addresses, rather than relying on the code that's adding entries to either of those sections to also handle the rebasing. I will try that... however, there isn't a funnel point that will cover absolute addresses from both our SyntheticSections as well as from InputSections.

Sep 9 2020, 6:16 PM · Restricted Project

Sep 8 2020

smeenai added inline comments to D86805: [lld-macho] create __TEXT,__unwind_info from __LD,__compact_unwind.
Sep 8 2020, 5:17 PM · Restricted Project
smeenai accepted D86908: [lld-macho] Mark weak symbols in symbol table.

LGTM

Sep 8 2020, 1:47 PM · Restricted Project

Aug 28 2020

smeenai added a comment to D86762: [ELF] Add documentation for --warn-backrefs: a GNU ld compatibility checking tool (and lesser of layering detection).

Is there a more specific term than "traditional linker"?

I understand that LLD's search strategy is uncommon among linkers in the Posix (and/or ELF?) realm, but it's not uncommon in other linkers, even very old ones. For example, the VMS (now OpenVMS) linker has used LLD's strategy by default for many decades. (It does allow opting in to the more restrictive search.) LLD's approach matches Microsoft's link.exe. I can't be sure, but I don't recall the so-called traditional behavior from linkers from other vendors of Windows, DOS, or CP/M linkers.

I have no objection to turning the warning on in appropriate contexts or with the argument that it helps to ensure good layering. I'm just quibbling over the term "traditional."

Aug 28 2020, 2:17 PM · Restricted Project

Aug 27 2020

smeenai accepted D86749: [lld-macho][NFC] Define isHidden() in LinkEditSection.

LGTM

Aug 27 2020, 4:47 PM · Restricted Project
smeenai accepted D86746: [lld-macho] Weak locals should be relaxed too.

LGTM; good catch.

Aug 27 2020, 4:11 PM · Restricted Project
smeenai accepted D86575: [lld-macho] Emit binding opcodes for defined symbols that override weak dysyms.

LGTM

Aug 27 2020, 2:24 PM · Restricted Project
smeenai accepted D86728: [lld-macho] Disable invalid/stub-link.s test for Mac.
Aug 27 2020, 11:22 AM · Restricted Project
smeenai accepted D86573: [lld-macho] Implement weak binding for branch relocations.

LGTM

Aug 27 2020, 10:56 AM · Restricted Project

Aug 26 2020

smeenai accepted D86642: [lld-macho] Support GOT relocations to __dso_handle.
Aug 26 2020, 10:01 PM · Restricted Project
smeenai accepted D86641: [lld-macho] Implement GOT_LOAD relaxation.

LGTM

Aug 26 2020, 9:59 PM · Restricted Project
smeenai added inline comments to D86575: [lld-macho] Emit binding opcodes for defined symbols that override weak dysyms.
Aug 26 2020, 9:54 PM · Restricted Project
smeenai accepted D86574: [lld-macho] Emit the right header flags for weak bindings/symbols.

LGTM

Aug 26 2020, 9:35 PM · Restricted Project
smeenai accepted D86573: [lld-macho] Implement weak binding for branch relocations.

LGTM

Aug 26 2020, 9:21 PM · Restricted Project
smeenai accepted D86572: [lld-macho] Implement weak bindings for GOT/TLV.

LGTM

Aug 26 2020, 4:54 PM · Restricted Project
smeenai accepted D86640: [lld-macho] Implement -all_load.

LGTM

Aug 26 2020, 4:14 PM · Restricted Project
smeenai accepted D86571: [lld-macho][NFC] Handle GOT bindings and regular bindings more uniformly.

LGTM; this is a nice cleanup.

Aug 26 2020, 4:02 PM · Restricted Project
smeenai accepted D85404: [lld-macho] Handle TAPI and regular re-exports uniformly.

LGTM. Thanks for limiting re-exports to sub-documents in the same TBD!

Aug 26 2020, 2:05 PM · Restricted Project
smeenai accepted D86180: [lld-macho] Make it possible to re-export .tbd files.

LGTM

Aug 26 2020, 1:46 PM · Restricted Project
smeenai added a comment to D86640: [lld-macho] Implement -all_load.

You're missing a test case.

Aug 26 2020, 1:42 PM · Restricted Project
smeenai accepted D86181: [lld-macho] Implement -ObjC.

LGTM

Aug 26 2020, 1:41 PM · Restricted Project

Aug 25 2020

smeenai added a comment to D86359: [llvm-libtool-darwin] Add support for -V option.

It looks like no documentation was added for the new option. Please add it!

Aug 25 2020, 3:04 PM · Restricted Project
smeenai committed rG22cd6bee4a7d: [llvm-libtool-darwin] Address post-commit feedback (authored by smeenai).
[llvm-libtool-darwin] Address post-commit feedback
Aug 25 2020, 3:04 PM

Aug 24 2020

smeenai committed rG2c80e2fe51b6: [runtimes] Use llvm-libtool-darwin for runtimes build (authored by smeenai).
[runtimes] Use llvm-libtool-darwin for runtimes build
Aug 24 2020, 1:49 PM
smeenai committed rGa7d8aabf298c: [runtimes] Remove TOOLCHAIN_TOOLS specialization (authored by smeenai).
[runtimes] Remove TOOLCHAIN_TOOLS specialization
Aug 24 2020, 1:49 PM
smeenai committed rG68bae34c65b3: [llvm-libtool-darwin] Add support for -V option (authored by smeenai).
[llvm-libtool-darwin] Add support for -V option
Aug 24 2020, 1:49 PM
smeenai committed rG26c1d689ae4c: [compiler-rt] Disable ranlib when using libtool (authored by smeenai).
[compiler-rt] Disable ranlib when using libtool
Aug 24 2020, 1:49 PM
smeenai closed D86367: [runtimes] Use llvm-libtool-darwin for runtimes build.
Aug 24 2020, 1:49 PM · Restricted Project, Restricted Project
smeenai closed D86366: [runtimes] Remove TOOLCHAIN_TOOLS specialization.
Aug 24 2020, 1:49 PM · Restricted Project
smeenai closed D86365: [compiler-rt] Disable ranlib when using libtool.
Aug 24 2020, 1:49 PM · Restricted Project
smeenai closed D86359: [llvm-libtool-darwin] Add support for -V option.
Aug 24 2020, 1:49 PM · Restricted Project
smeenai added a comment to D85740: Universal MachO: support LLVM IR objects.

I think llvm-libtool-darwin would also need some adjustments to handle bitcode objects, archives, and universal binaries. That's definitely out of scope for this diff, but I'm just flagging it as another tool with missing bitcode support.

Aug 24 2020, 10:52 AM · Restricted Project

Aug 21 2020

smeenai requested review of D86367: [runtimes] Use llvm-libtool-darwin for runtimes build.
Aug 21 2020, 1:10 PM · Restricted Project, Restricted Project
smeenai requested review of D86366: [runtimes] Remove TOOLCHAIN_TOOLS specialization.
Aug 21 2020, 1:10 PM · Restricted Project
smeenai requested review of D86365: [compiler-rt] Disable ranlib when using libtool.
Aug 21 2020, 1:09 PM · Restricted Project
smeenai updated subscribers of D86263: [ELF] Keep st_type for symbol assignment.

@hans, would you consider this for 11.0? 11.0 LLD will no longer perform ARM-Thumb interworking for symbols that aren't marked STT_FUNC, so this fixes cases where interworking would incorrectly not occur because of a simple alias.

Aug 21 2020, 11:00 AM · Restricted Project
smeenai requested review of D86359: [llvm-libtool-darwin] Add support for -V option.
Aug 21 2020, 10:54 AM · Restricted Project

Aug 20 2020

smeenai committed rG16f27e1e18fd: [cmake] Don't use ld.lld when targeting Darwin (authored by smeenai).
[cmake] Don't use ld.lld when targeting Darwin
Aug 20 2020, 7:52 PM
smeenai committed rGe2ab5bcf5691: [runtimes] Allow LLVM_BUILTIN_TARGETS to include Darwin (authored by smeenai).
[runtimes] Allow LLVM_BUILTIN_TARGETS to include Darwin
Aug 20 2020, 6:29 PM
smeenai closed D86313: [runtimes] Allow LLVM_BUILTIN_TARGETS to include Darwin.
Aug 20 2020, 6:29 PM · Restricted Project
smeenai added a comment to D86313: [runtimes] Allow LLVM_BUILTIN_TARGETS to include Darwin.

I agree that the current situation is not optimal, I was never happy with the default case which is confusing for a lot of people I talked to. I would like to come up with a better solution, but I'm not sure if special casing the x86_64-apple-darwin is sufficient though. With Apple Silicon, I assume we're going to be building runtimes as universal binaries that target multiple architectures.

We could consider completely decoupling runtimes from targets. Originally, each runtime build was identified only by its target, so you'd use RUNTIMES_${target}_${variable} to set target-specific variables. That became insufficient when we introduced the support for multilib, so now you also have a name and you can use RUNTIMES_${target}+${name}_${variable}. We could switch to RUNTIMES_${name}_${variable} and it'd be the responsibility of the user to set the target explicitly if they want it (FWIW it's something we already have to do in our build anyway because of Windows where the CMake behavior when you set CMAKE_${LANG}_COMPILER_TARGET breaks the build). If not, they get the default behavior, matching the default case today. What do you think?

To clarify, what would be setting ${name} in this scenario?

It'd be set by user. Today we have BUILTIN_TARGETS and RUNTIME_TARGETS, we could either repurpose those and create new variables. You'd use it as follows:

list(APPEND BUILTIN_TARGETS "darwin")
set(BUILTINS_darwin_CMAKE_SYSTEM_NAME Darwin CACHE STRING "")
...
list(APPEND BUILTIN_TARGETS "linux")
set(BUILTINS_linux_CMAKE_SYSTEM_NAME Linux CACHE STRING "")
set(BUILTINS_linux_CMAKE_C_COMPILER_TARGET x86_64-linux-gnu CACHE STRING "")
...

Here darwin and linux are arbitrary names.

Aug 20 2020, 5:06 PM · Restricted Project
smeenai updated the diff for D86313: [runtimes] Allow LLVM_BUILTIN_TARGETS to include Darwin.

Check LLVM_RUNTIME_TARGETS as well

Aug 20 2020, 3:30 PM · Restricted Project
smeenai added a comment to D86313: [runtimes] Allow LLVM_BUILTIN_TARGETS to include Darwin.

Okay, I would argue that this version of the change is much better than the current situation. In particular, it disallows the loophole of specifying a triple for a particular platform instead of "darwin", and it forces users of "darwin" to acknowledge they understand how the Darwin compiler-rt build works by setting a particular variable (and it still disallows multiple Darwin triples). Lemme know what you think.

Aug 20 2020, 2:13 PM · Restricted Project
smeenai updated the diff for D86313: [runtimes] Allow LLVM_BUILTIN_TARGETS to include Darwin.

Add explicit error and flag

Aug 20 2020, 2:11 PM · Restricted Project
smeenai added a comment to D86313: [runtimes] Allow LLVM_BUILTIN_TARGETS to include Darwin.

I think this change makes configuration confusingly inaccurate. Since the compiler-rt Darwin build does not support picking and choosing which targets to configure making the configuration fail if a user tries to is the reasonable approach.

Making the build do something different and unintuitive from what the configuration requested seems undesirable to me.

Aug 20 2020, 1:17 PM · Restricted Project
smeenai requested review of D86313: [runtimes] Allow LLVM_BUILTIN_TARGETS to include Darwin.
Aug 20 2020, 12:45 PM · Restricted Project
smeenai added a comment to D86263: [ELF] Keep st_type for symbol assignment.

Thank you!

Aug 20 2020, 9:25 AM · Restricted Project

Aug 16 2020

smeenai committed rG402b063c8067: [llvm-libtool-darwin] Fix test on all host architectures (authored by smeenai).
[llvm-libtool-darwin] Fix test on all host architectures
Aug 16 2020, 12:18 AM
smeenai added a comment to D85334: [llvm-libtool-darwin] Support universal outputs.

rG402b063c8067 should fix it. Sorry for the breakage!

Aug 16 2020, 12:18 AM · Restricted Project
smeenai added a comment to D85334: [llvm-libtool-darwin] Support universal outputs.

That didn't work, but I'm able to repro locally using the CMake cache file used by the builder, so I'm digging into it.

Aug 16 2020, 12:09 AM · Restricted Project

Aug 15 2020

smeenai added a comment to D85334: [llvm-libtool-darwin] Support universal outputs.

I pushed rG12b4df991950 as a speculative fix. I'll monitor the bot.

Aug 15 2020, 9:34 PM · Restricted Project