Page MenuHomePhabricator

steven_wu (Steven Wu)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 4 2014, 3:46 PM (394 w, 2 d)

Recent Activity

Yesterday

steven_wu accepted D125974: [clang] Limit bitcode option ignorelist to Darwin.

Thanks! LGTM.

Thu, May 26, 2:22 PM · Restricted Project, Restricted Project

Fri, May 20

steven_wu added a comment to D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types.

I'm still looking into a whole program devirtualization bug that only repros with opaque pointers, and there are still potential performance issues to look into

Fri, May 20, 11:18 AM · Restricted Project, Restricted Project, Restricted Project
steven_wu added a comment to D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types.

Is there any issues currently if we just always use opaque pointer in LTO?

Fri, May 20, 10:54 AM · Restricted Project, Restricted Project, Restricted Project

Thu, May 19

steven_wu added a comment to D125974: [clang] Limit bitcode option ignorelist to Darwin.

LGTM. Can you add a test for your usecase?

Thu, May 19, 11:10 AM · Restricted Project, Restricted Project

Tue, May 17

steven_wu added a comment to D125478: [llvm-objcopy][test] Add cmp after copy.

This breaks darwin bot: https://green.lab.llvm.org/green/job/clang-stage1-RA/29421/

Tue, May 17, 8:41 AM · Restricted Project, Restricted Project

Tue, May 3

steven_wu added inline comments to D124557: [compiler-rt][Darwin] Check for arm64 support directly.
Tue, May 3, 11:17 AM · Restricted Project, Restricted Project

Apr 20 2022

steven_wu added a comment to D124106: [lld/mac] Warn that writing zippered outputs isn't implemented.

Maybe it is easier if you only warn if multiple different platforms are specified? You can preserve the last one wins for the same platform.
Or even more specifically, the only supported configuration is macOS + macCatalyst and you can only toss out a warning if this configuration is used.

Apr 20 2022, 10:22 AM · Restricted Project, Restricted Project, Restricted Project

Apr 19 2022

steven_wu added a reviewer for D124010: [llvm-cxxfilt] Unable to demangle a template argument which happens to be a null pointer: ldionne.
Apr 19 2022, 9:08 AM · Restricted Project, Restricted Project, Restricted Project

Mar 30 2022

steven_wu accepted D122700: Do not generate x86_64 test targets on AS hots.
Mar 30 2022, 11:20 AM · Restricted Project, Restricted Project

Mar 15 2022

steven_wu committed rGe168513aed8f: [ASAN] Fix darwin-interface test (authored by steven_wu).
[ASAN] Fix darwin-interface test
Mar 15 2022, 6:14 AM · Restricted Project
steven_wu closed D121636: [ASAN] Fix darwin-interface test.
Mar 15 2022, 6:14 AM · Restricted Project, Restricted Project

Mar 14 2022

steven_wu updated the diff for D121636: [ASAN] Fix darwin-interface test.

Add win32.

Mar 14 2022, 3:50 PM · Restricted Project, Restricted Project
steven_wu added a comment to D121636: [ASAN] Fix darwin-interface test.

Not sure about win32 situation as it is also guarded in compiler-rt/lib/asan/CMakeLists.txt:

if (NOT WIN32 AND NOT APPLE)
   list(APPEND ASAN_SOURCES
     asan_rtl_x86_64.S
     asan_interceptors_vfork.S
     )
 endif()
Mar 14 2022, 1:41 PM · Restricted Project, Restricted Project
steven_wu requested review of D121636: [ASAN] Fix darwin-interface test.
Mar 14 2022, 1:40 PM · Restricted Project, Restricted Project
steven_wu closed D121608: [CodeGen] Fix implicit module build.

This is actually fixed in f51d7e4bae9e861e711ad9711599456fc2f1bbca before this is reviewed. I will just close this.

Mar 14 2022, 12:39 PM · Restricted Project, Restricted Project
steven_wu requested review of D121608: [CodeGen] Fix implicit module build.
Mar 14 2022, 9:13 AM · Restricted Project, Restricted Project
steven_wu added a comment to D121332: Cleanup includes: DebugInfo & CodeGen.

This breaks implicit module on macOS bots: https://green.lab.llvm.org/green/job/lldb-cmake/42098/console

Mar 14 2022, 9:03 AM · Restricted Project, Restricted Project, Restricted Project

Mar 9 2022

steven_wu added a comment to D118352: [clang][ABI] New c++20 modules mangling scheme.

We're also seeing this issue on our Mac bots, is it possible to revert it?

I've reverted it. Since this is the new feature, I think it wouldn't so hurry to land it. BTW, I think it would be helpful to provide more information to fix it.

Mar 9 2022, 9:03 AM · Restricted Project, Restricted Project

Mar 3 2022

steven_wu added a comment to D105283: [VP] Introducing VectorBuilder, the VP intrinsic builder.

Just a note, this also breaks implicit module build as the newly added header indirectly includes llvm/IR/Attributes.inc which needs to be generated first. If you do a module build (-DLLVM_ENABLE_MODULES=ON), you are likely to get error:

09:05:38 While building module 'LLVM_MC' imported from /System/Volumes/Data/jenkins/workspace/apple-clang-stage2-configure-RA_master/llvm-project/llvm/lib/MC/MCAsmBackend.cpp:9:
09:05:38 While building module 'LLVM_IR' imported from /System/Volumes/Data/jenkins/workspace/apple-clang-stage2-configure-RA_master/llvm-project/llvm/include/llvm/MC/MCPseudoProbe.h:49:
09:05:38 While building module 'LLVM_intrinsic_gen' imported from /System/Volumes/Data/jenkins/workspace/apple-clang-stage2-configure-RA_master/llvm-project/llvm/include/llvm/IR/VectorBuilder.h:18:
09:05:38 In file included from <module-includes>:1:
09:05:38 In file included from /System/Volumes/Data/jenkins/workspace/apple-clang-stage2-configure-RA_master/llvm-project/llvm/include/llvm/IR/Argument.h:17:
09:05:38 /System/Volumes/Data/jenkins/workspace/apple-clang-stage2-configure-RA_master/llvm-project/llvm/include/llvm/IR/Attributes.h:75:14: fatal error: 'llvm/IR/Attributes.inc' file not found
09:05:38     #include "llvm/IR/Attributes.inc"
09:05:38              ^~~~~~~~~~~~~~~~~~~~~~~~
09:05:38 While building module 'LLVM_MC' imported from /System/Volumes/Data/jenkins/workspace/apple-clang-stage2-configure-RA_master/llvm-project/llvm/lib/MC/MCAsmBackend.cpp:9:
09:05:38 While building module 'LLVM_IR' imported from /System/Volumes/Data/jenkins/workspace/apple-clang-stage2-configure-RA_master/llvm-project/llvm/include/llvm/MC/MCPseudoProbe.h:49:
09:05:38 In file included from <module-includes>:62:
09:05:38 /System/Volumes/Data/jenkins/workspace/apple-clang-stage2-configure-RA_master/llvm-project/llvm/include/llvm/IR/VectorBuilder.h:18:10: fatal error: could not build module 'LLVM_intrinsic_gen'
09:05:38 #include <llvm/IR/IRBuilder.h>
09:05:38  ~~~~~~~~^
Mar 3 2022, 1:02 PM · Restricted Project, Restricted Project, Restricted Project

Feb 8 2022

steven_wu accepted D118862: [clang][driver] add clang driver support for emitting macho files with two build version load commands.

LGTM

Feb 8 2022, 9:57 AM · Restricted Project
steven_wu added a comment to D72404: [ThinLTO/FullLTO] Support Os and Oz.

I just saw this. I know it is a good idea to have choice during link time for the pipeline configuration and from your benchmark it also has size impact on the output. I also feel like this is going in the wrong direction as if I have part of the object files built with -O3 and part built with -Oz, I need to make a choice of unoptimized part of the program.

Feb 8 2022, 9:56 AM · Restricted Project, Restricted Project

Jan 18 2022

steven_wu committed rG091e364866fb: [JITLink][ELF] Support duplicated section names from object file (authored by steven_wu).
[JITLink][ELF] Support duplicated section names from object file
Jan 18 2022, 8:39 AM
steven_wu closed D114753: [JITLink][ELF] Support duplicated section names from object file.
Jan 18 2022, 8:38 AM · Restricted Project
steven_wu committed rG347d4d7323c4: [ADT] Fix Optional<> with llvm::is_trivially_move_constructible (authored by steven_wu).
[ADT] Fix Optional<> with llvm::is_trivially_move_constructible
Jan 18 2022, 8:38 AM
steven_wu closed D117254: [ADT] Fix Optional<> with llvm::is_trivially_move_constructible.
Jan 18 2022, 8:37 AM · Restricted Project

Jan 17 2022

steven_wu updated the diff for D117254: [ADT] Fix Optional<> with llvm::is_trivially_move_constructible.

clang-format

Jan 17 2022, 9:29 PM · Restricted Project

Jan 14 2022

steven_wu updated the diff for D117254: [ADT] Fix Optional<> with llvm::is_trivially_move_constructible.

Rebase to TOT

Jan 14 2022, 2:43 PM · Restricted Project
steven_wu updated the diff for D114753: [JITLink][ELF] Support duplicated section names from object file.

Rebase the change to TOT

Jan 14 2022, 2:36 PM · Restricted Project

Jan 13 2022

steven_wu added inline comments to D117254: [ADT] Fix Optional<> with llvm::is_trivially_move_constructible.
Jan 13 2022, 5:22 PM · Restricted Project
steven_wu updated the diff for D117254: [ADT] Fix Optional<> with llvm::is_trivially_move_constructible.

Fix test name.

Jan 13 2022, 5:20 PM · Restricted Project
steven_wu updated the diff for D117254: [ADT] Fix Optional<> with llvm::is_trivially_move_constructible.

Address review feedback.

Jan 13 2022, 3:47 PM · Restricted Project
steven_wu updated the diff for D117254: [ADT] Fix Optional<> with llvm::is_trivially_move_constructible.

Rename the variables to be more accurate.

Jan 13 2022, 2:39 PM · Restricted Project
steven_wu requested review of D117254: [ADT] Fix Optional<> with llvm::is_trivially_move_constructible.
Jan 13 2022, 2:35 PM · Restricted Project

Dec 13 2021

steven_wu accepted D114201: [LTO] Add a function `LTOCodeGenerator::getMergedModule`.

Thanks! LGTM now.

Dec 13 2021, 9:16 AM · Restricted Project

Dec 10 2021

steven_wu accepted D115415: [clang][macho] add clang frontend support for emitting macho files with two build version load commands.
Dec 10 2021, 9:09 AM · Restricted Project, Restricted Project

Dec 1 2021

steven_wu added a comment to D114753: [JITLink][ELF] Support duplicated section names from object file.

@lhames ping

Dec 1 2021, 10:38 AM · Restricted Project

Nov 30 2021

steven_wu committed rGc737d4d20321: [JITLink][ELF] Don't skip sections of size 0 (authored by steven_wu).
[JITLink][ELF] Don't skip sections of size 0
Nov 30 2021, 9:19 AM
steven_wu committed rG5e3200f3ce5a: [JITLink][ELF] Add support for reading extended table (authored by steven_wu).
[JITLink][ELF] Add support for reading extended table
Nov 30 2021, 9:19 AM
steven_wu closed D114749: [JITLink][ELF] Don't skip sections of size 0.
Nov 30 2021, 9:19 AM · Restricted Project
steven_wu closed D114747: [JITLink][ELF] Add support for reading extended table.
Nov 30 2021, 9:19 AM · Restricted Project
steven_wu updated the diff for D114753: [JITLink][ELF] Support duplicated section names from object file.

Fix the rebase issue that mixed up with D114749

Nov 30 2021, 9:16 AM · Restricted Project
steven_wu updated the diff for D114749: [JITLink][ELF] Don't skip sections of size 0.

Address review feedback

Nov 30 2021, 9:14 AM · Restricted Project
steven_wu added inline comments to D114749: [JITLink][ELF] Don't skip sections of size 0.
Nov 30 2021, 9:11 AM · Restricted Project
steven_wu updated the diff for D114753: [JITLink][ELF] Support duplicated section names from object file.

Address review feedback

Nov 30 2021, 9:10 AM · Restricted Project

Nov 29 2021

steven_wu requested review of D114753: [JITLink][ELF] Support duplicated section names from object file.
Nov 29 2021, 3:36 PM · Restricted Project
steven_wu requested review of D114749: [JITLink][ELF] Don't skip sections of size 0.
Nov 29 2021, 3:26 PM · Restricted Project
steven_wu requested review of D114747: [JITLink][ELF] Add support for reading extended table.
Nov 29 2021, 3:16 PM · Restricted Project
steven_wu accepted D114683: [LTO] Specify triple to address unknown binary format assertion.

LGTM.

Nov 29 2021, 9:04 AM · Restricted Project
steven_wu requested changes to D114201: [LTO] Add a function `LTOCodeGenerator::getMergedModule`.

Few things to improve before commit:

  • Drop the words like "in my case", "I", etc. from the commit message. Eg, In my case, after optimize is called, I need to access the underlying module to get information. can be rewritten as API can be used to access the underlying module to get information after optimized is called. It would sound much better.
  • Please add a use-case of the API in test suite otherwise it can be removed as dead code. You can look into getting it used in llvm-lto tool and add a testcase to exercise the code path.
Nov 29 2021, 9:01 AM · Restricted Project

Nov 19 2021

steven_wu added a comment to D114201: [LTO] Add a function `LTOCodeGenerator::getMergedModule`.

The change is fine. Can you elaborate a bit more about your use case? Or at least add a test for it?

Nov 19 2021, 10:58 AM · Restricted Project

Nov 15 2021

steven_wu committed rGfcd07f810781: [JITLink] Fix splitBlock if there are symbols span across the boundary (authored by steven_wu).
[JITLink] Fix splitBlock if there are symbols span across the boundary
Nov 15 2021, 1:56 PM
steven_wu closed D113912: [JITLink] Fix splitBlock if there are symbols span across the boundary.
Nov 15 2021, 1:55 PM · Restricted Project
steven_wu updated the diff for D113912: [JITLink] Fix splitBlock if there are symbols span across the boundary.

Update the patch to trust user's action and update symbols to fit in the new block

Nov 15 2021, 12:52 PM · Restricted Project
steven_wu updated the diff for D113912: [JITLink] Fix splitBlock if there are symbols span across the boundary.

Address review feedback after talking with Lang and Duncan offline.

Nov 15 2021, 11:59 AM · Restricted Project
steven_wu added inline comments to D113912: [JITLink] Fix splitBlock if there are symbols span across the boundary.
Nov 15 2021, 10:46 AM · Restricted Project
steven_wu requested review of D113912: [JITLink] Fix splitBlock if there are symbols span across the boundary.
Nov 15 2021, 9:38 AM · Restricted Project

Oct 29 2021

steven_wu added inline comments to D112297: [LTO] Fix assertion failed when flushing bitcode incrementally for LTO output..
Oct 29 2021, 12:27 PM · Restricted Project

Oct 27 2021

steven_wu accepted D112649: [libunwind] Simplify the executor used in the tests.
Oct 27 2021, 12:46 PM · Restricted Project, Restricted Project

Oct 22 2021

steven_wu accepted D112322: [libunwind] Fix path to libunwind for per-target-runtime-dir builds.

LGTM. Can you use a better commit message?

Oct 22 2021, 9:12 AM · Restricted Project, Restricted Project
steven_wu added a reviewer for D112124: [ThinLTO] Append original object file name to ThinLTO generated object file's name, so that it can be recorded in link map file.: davide.

The best solution might still be the linker to map the intermediate file to the original object file since the added file name here is still another level of indirection you need to figure out. @davide for his opinion.

Oct 22 2021, 8:40 AM · Restricted Project

Oct 21 2021

steven_wu accepted D112189: [macho] add support for emitting macho files with two build version load commands.
Oct 21 2021, 10:57 AM · Restricted Project

Oct 18 2021

steven_wu added a comment to D111863: [libunwind] Add an interface for dynamic .eh_frame registration.

I think the ORC runtime provides a much more natural way to test this. Did you manage to come up with some ORC-runtime based tests in the end?

My current plan is to automate what I've been doing manually, namely running a test of C++ exception handling using llvm-jitlink, both with the default configuration (using libgcc-provided unwinding), and with libunwind LD_PRELOADed to force it as the unwinding provider.

Oct 18 2021, 1:28 PM · Restricted Project, Restricted Project
steven_wu added a comment to D111863: [libunwind] Add an interface for dynamic .eh_frame registration.

I don't know enough about Dwarf unwinding but the implementation looks generally good. Can you please add a testcase indicating how ORCJIT is planning to use it?

Oct 18 2021, 10:02 AM · Restricted Project, Restricted Project

Oct 7 2021

steven_wu added a reviewer for D111164: Regenerate LC_CODE_SIGNATURE during llvm-objcopy operations: davide.
Oct 7 2021, 9:42 AM · Restricted Project, Restricted Project, Restricted Project

Sep 27 2021

steven_wu accepted D110075: [LTO] [Legacy] Add -debug-pass-manager option to enable pass run/skip trace..
Sep 27 2021, 11:38 AM · Restricted Project

Sep 23 2021

steven_wu accepted D110369: Support: Skip buffering buffer_unique_ostream's owned stream.

LGTM.

Sep 23 2021, 4:54 PM · Restricted Project

Sep 21 2021

steven_wu added inline comments to D109972: Regenerate LC_CODE_SIGNATURE during llvm-objcopy operations.
Sep 21 2021, 1:17 PM · Restricted Project
steven_wu added a comment to D109972: Regenerate LC_CODE_SIGNATURE during llvm-objcopy operations.

+ @davide who might have more context of LC_CODE_SIGNITURE section

Sep 21 2021, 11:58 AM · Restricted Project
steven_wu added a reviewer for D109972: Regenerate LC_CODE_SIGNATURE during llvm-objcopy operations: davide.
Sep 21 2021, 11:50 AM · Restricted Project

Sep 20 2021

steven_wu accepted D88458: [CMake] Cache the compiler-rt library search results.

I want to do something like this for a long time. It would be good to ReleaseNote the change so people who is distributing/building with a subset of the project directories can be updated to build with top level cmake directory (@ldionne that probably include our internal build system). Some day we might be able to cmake configure with the top level llvm-project directory.

Sep 20 2021, 3:38 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
steven_wu added a comment to D110075: [LTO] [Legacy] Add -debug-pass-manager option to enable pass run/skip trace..

I assume this is a debug option for LTO developer that is not going for production. That is why I said LGTM without a test case. I do think it will be good to set that in ThinLTOCodeGenerator as well, probably not needed for llvm-lto2.

Sep 20 2021, 3:31 PM · Restricted Project
steven_wu accepted D110075: [LTO] [Legacy] Add -debug-pass-manager option to enable pass run/skip trace..

LGTM.

Sep 20 2021, 9:06 AM · Restricted Project

Sep 9 2021

steven_wu committed rG05eaa2b42f66: [CMake][Darwin] Ignore stderr during SDKSetting.plist parsing (authored by steven_wu).
[CMake][Darwin] Ignore stderr during SDKSetting.plist parsing
Sep 9 2021, 12:10 PM
steven_wu closed D108156: [CMake][Darwin] Ignore stderr during SDKSetting.plist parsing.
Sep 9 2021, 12:10 PM · Restricted Project

Sep 8 2021

steven_wu accepted D108881: [clang][driver] Honor the last -flto(=.*)? argument.

I am not familiar with offloading side of LTO driver but the change looks functionally the same to me. Thanks for doing this! Remember to update the commit message with the correct info.

Sep 8 2021, 9:32 AM · Restricted Project

Sep 7 2021

steven_wu added a comment to D108881: [clang][driver] Honor the last -flto(=.*)? argument.

I will do a cleanup of parseLTOMode function since we don't need a OptPos parameter anymore. There are few minor places references OPT_flto or OPT_foffload_lto can be cleaned up too.

Will you incorporate the functional changes in this patch? Or is there still a need for this change?

The current change set in this review is functional change while the cleanup I want is not functional after the rewrite the old option as Alias. Once flto is the alias, there is no need to handle that in the driver and those might actually become source of bug in the future.

I think it would be good to do the cleanup in the same commit unless you have compelling reason not to.

Hi @steven_wu any idea about the timeline?

This issue is blocking some internal work, and assuming that it will take longer to get a full fix, I would prefer it if this change could go in on its own.
Otherwise I am good with doing everything in the same commit.

I expect the cleanup is very very simple and that is why I am suggested to do in the same commit. I am ok with a followup fix as well.

I think there might be some high level confusion. @steven_wu earlier in the thread you said "I will do a cleanup", but I think you were asking @mnadeem to do the cleanup here in this patch.

Sep 7 2021, 2:56 PM · Restricted Project
steven_wu added a comment to D108881: [clang][driver] Honor the last -flto(=.*)? argument.

If you didn't quite get what I mean, I am suggesting to cleanup all the reference to the old flag. Since -flto is rewrite by the driver to -flto=full, clang should not query OPT_flto anymore. All references to the old flag needs to be audited and removed while preserving the correct behavior.

Sep 7 2021, 2:53 PM · Restricted Project
steven_wu added a comment to D108881: [clang][driver] Honor the last -flto(=.*)? argument.

I will do a cleanup of parseLTOMode function since we don't need a OptPos parameter anymore. There are few minor places references OPT_flto or OPT_foffload_lto can be cleaned up too.

Will you incorporate the functional changes in this patch? Or is there still a need for this change?

The current change set in this review is functional change while the cleanup I want is not functional after the rewrite the old option as Alias. Once flto is the alias, there is no need to handle that in the driver and those might actually become source of bug in the future.

I think it would be good to do the cleanup in the same commit unless you have compelling reason not to.

Hi @steven_wu any idea about the timeline?

This issue is blocking some internal work, and assuming that it will take longer to get a full fix, I would prefer it if this change could go in on its own.
Otherwise I am good with doing everything in the same commit.

Sep 7 2021, 2:51 PM · Restricted Project

Aug 30 2021

steven_wu added a comment to D108881: [clang][driver] Honor the last -flto(=.*)? argument.

I will do a cleanup of parseLTOMode function since we don't need a OptPos parameter anymore. There are few minor places references OPT_flto or OPT_foffload_lto can be cleaned up too.

Will you incorporate the functional changes in this patch? Or is there still a need for this change?

Aug 30 2021, 6:34 PM · Restricted Project
steven_wu added a comment to D108881: [clang][driver] Honor the last -flto(=.*)? argument.

I will do a cleanup of parseLTOMode function since we don't need a OptPos parameter anymore. There are few minor places references OPT_flto or OPT_foffload_lto can be cleaned up too.

Aug 30 2021, 4:19 PM · Restricted Project
steven_wu added a comment to D108881: [clang][driver] Honor the last -flto(=.*)? argument.

I agree that it might be good to change to the behavior that last arg wins but it might be simpler to set -flto to be an alias of -flto=full:

--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2086,7 +2086,7 @@ def flto_EQ : Joined<["-"], "flto=">, Flags<[CoreOption, CC1Option]>, Group<f_Gr
 def flto_EQ_jobserver : Flag<["-"], "flto=jobserver">, Group<f_Group>;
 def flto_EQ_auto : Flag<["-"], "flto=auto">, Group<f_Group>;
 def flto : Flag<["-"], "flto">, Flags<[CoreOption, CC1Option]>, Group<f_Group>,
-  HelpText<"Enable LTO in 'full' mode">;
+  Alias<flto_EQ>, AliasArgs<["full"]>, HelpText<"Enable LTO in 'full' mode">;
 def fno_lto : Flag<["-"], "fno-lto">, Flags<[CoreOption, CC1Option]>, Group<f_Group>,
   HelpText<"Disable LTO mode (default)">;
 def foffload_lto_EQ : Joined<["-"], "foffload-lto=">, Flags<[CoreOption]>, Group<f_Group>,
Aug 30 2021, 1:52 PM · Restricted Project

Aug 16 2021

steven_wu requested review of D108156: [CMake][Darwin] Ignore stderr during SDKSetting.plist parsing.
Aug 16 2021, 12:10 PM · Restricted Project

Aug 12 2021

steven_wu added a comment to D107799: [CMake] Enable LLVM_ENABLE_PER_TARGET_RUNTIME_DIR by default on Linux.

Is there any reason why the prefix couldn't be lib/clang/14/ instead of lib/clang/14.0.0/ ?

I think keeping the full version number is good in case we rev any of the headers or libraries shipped with clang in a patch release. It is good to be able to ensure those directories don't overlap or have stale contents if clang gets installed into the same root directory.

That would better match other platforms, but I'd like to hear from Apple folks like @beanz whether this is something that would make sense.

I'm probably not the best person from Apple to weigh in here since I'm not actively involved with clang toolchains anymore. @steven_wu & @arphaman are probably better people to consult since they'd need to be involved.

I always thought it would be best to get Darwin to start building thin-archives for all the compiler-rt libraries, and to move to triple-based directory structure. There's no reason the compiler can't operate on thin files, and it would allow much of the driver and build system code to become unified. Which would be a huge win.

I suspect part of how we got to the place we're at now is because the driver doesn't share a lot of code between targets, and back in the old makefile days each target basically had its own makefiles for compiler-rt. CMake always tried to share logic, but became a mess trying to do _very_ different things.

It would be a big project to clean up and unify all of that, but totally worth doing.

Aug 12 2021, 1:52 PM · Restricted Project, Restricted Project
steven_wu accepted D107667: [clang/test] Run thinlto-clang-diagnostic-handler-in-be.c on x86.

LGTM. Thanks

Aug 12 2021, 12:37 PM · Restricted Project

Aug 9 2021

steven_wu added a comment to D107667: [clang/test] Run thinlto-clang-diagnostic-handler-in-be.c on x86.

Please don't use x86_64 as target or triple since it might default to some weird platform. Can you spell out a valid triple/platform?

Aug 9 2021, 10:12 AM · Restricted Project
steven_wu added a comment to D107473: [LLVM][LTO][NFC] Resolve FIXME in ThinLTOCodeGenerator.cpp.

Can you unify the duplicated code or just put the FIXME back saying need to fix the duplicated code in the future?

Aug 9 2021, 10:09 AM · Restricted Project

Aug 4 2021

steven_wu added a comment to D107473: [LLVM][LTO][NFC] Resolve FIXME in ThinLTOCodeGenerator.cpp.

That is said, I am fine with the clean up but this probably doesn't resolve FIXME.

Aug 4 2021, 9:42 AM · Restricted Project
steven_wu added reviewers for D107473: [LLVM][LTO][NFC] Resolve FIXME in ThinLTOCodeGenerator.cpp: mehdi_amini, steven_wu.

I think the FIXME is about not duplicating code from LTOCodeGenerator, not how terrible if statement looks.

Aug 4 2021, 9:41 AM · Restricted Project

Jul 29 2021

steven_wu accepted D106316: [clang][darwin] Add support for the -mtargetos= option to the driver.
Jul 29 2021, 8:17 PM · Restricted Project

Jul 27 2021

steven_wu accepted D106887: [LTO][Legacy] Add new API to check presence of ctor/dtor functions.

LGTM in general. Just two small comments inline. Thanks!

Jul 27 2021, 2:42 PM · Restricted Project
steven_wu requested changes to D106887: [LTO][Legacy] Add new API to check presence of ctor/dtor functions.

Comment inline.

Jul 27 2021, 9:38 AM · Restricted Project

Jul 26 2021

steven_wu added a comment to D106305: [compiler-rt][CMake][arm64] Use a custom target for symlinking LSE sources.

Oh, wait, I do see this warning in the build and I am not sure if this will be a problem or not:

Dependee "/Users/steven/dev/monorepo/makefile_build/projects/compiler-rt/lib/builtins/CMakeFiles/lse_builtin_symlinks.dir/DependInfo.cmake" is newer than depender "/Users/steven/dev/monorepo/makefile_build/projects/compiler-rt/lib/builtins/CMakeFiles/lse_builtin_symlinks.dir/depend.internal".
Jul 26 2021, 9:43 AM · Restricted Project
steven_wu accepted D106305: [compiler-rt][CMake][arm64] Use a custom target for symlinking LSE sources.

Works now. Thanks!

Jul 26 2021, 9:39 AM · Restricted Project

Jul 25 2021

steven_wu added a comment to D106305: [compiler-rt][CMake][arm64] Use a custom target for symlinking LSE sources.

I still get the same error. 😿

Jul 25 2021, 6:13 PM · Restricted Project

Jul 23 2021

steven_wu added a comment to D106305: [compiler-rt][CMake][arm64] Use a custom target for symlinking LSE sources.

Now I got this error when generating makefile:

-- Configuring done
CMake Error at /Users/steven/dev/monorepo/llvm-project/compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake:275 (add_library):
  Cannot find source file:
Jul 23 2021, 8:35 AM · Restricted Project

Jul 22 2021

steven_wu added a comment to D106305: [compiler-rt][CMake][arm64] Use a custom target for symlinking LSE sources.

The error says ios_arm64e because it duplicates for every single os arm64 combination. osx_arm64, osx_arm64e, ios_arm64, ios_arm64e, etc.

Jul 22 2021, 8:35 AM · Restricted Project
steven_wu requested changes to D106305: [compiler-rt][CMake][arm64] Use a custom target for symlinking LSE sources.

No, this doesn't work:

CMake Error at /Users/steven/dev/monorepo/llvm-project/compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake:420 (add_custom_target):
  add_custom_target cannot create target "lse_builtin_symlinks" because
  another target with the same name already exists.  The existing target is a
  custom target created in source directory
  "/Users/steven/dev/monorepo/llvm-project/compiler-rt/lib/builtins".  See
  documentation for policy CMP0002 for more details.
Call Stack (most recent call first):
  /Users/steven/dev/monorepo/llvm-project/compiler-rt/lib/builtins/CMakeLists.txt:674 (darwin_add_builtin_libraries)
Jul 22 2021, 8:32 AM · Restricted Project

Jul 20 2021

steven_wu added a comment to D106305: [compiler-rt][CMake][arm64] Use a custom target for symlinking LSE sources.

The configured directory is rather large so let me give some more detail from the makefile generated. The custom target is generated in two of the makefiles I mentioned above, like following:

projects/compiler-rt/lib/builtins/outline_atomic_helpers.dir/outline_atomic_cas1_1.S:
        @$(CMAKE_COMMAND) -E cmake_echo_color --switch=$(COLOR) --blue --bold --progress-dir=/Users/steven/makefile_build/CMakeFiles --progress-num=$(CMAKE_PROGRESS_1) "Generating outline_atomic_helpers.dir/outline_atomic_cas1_1.S"
        cd /Users/steven/makefile_build/projects/compiler-rt/lib/builtins && /usr/local/bin/cmake -E create_symlink /Users/steven/llvm-project/compiler-rt/lib/builtins/aarch64/lse.S /Users/steven/makefile_build/projects/compiler-rt/lib/builtins/outline_atomic_helpers.dir/outline_atomic_cas1_1.S

When you build the clang builtin library, it does:

$ make -j 32 clang_rt.osx VERBOSE=1
...
/usr/bin/make  -f projects/compiler-rt/lib/builtins/CMakeFiles/clang_rt.builtins_arm64_osx.dir/build.make projects/compiler-rt/lib/        builtins/CMakeFiles/clang_rt.builtins_arm64_osx.dir/depend
/usr/bin/make  -f projects/compiler-rt/lib/builtins/CMakeFiles/clang_rt.builtins_arm64e_osx.dir/build.make projects/compiler-rt/lib/       builtins/CMakeFiles/clang_rt.builtins_arm64e_osx.dir/depend
...
CMake Error: failed to create symbolic link '/Users/steven/makefile_build/projects/compiler-rt/lib/builtins/outline_atomic_helpers.dir/outline_atomic_ldset1_3.S': file already exists
``

Thanks, I now understand why this issue happens. With Make each target gets a Make subinvocation, while in Ninja they're generated into a single buildfile and can be correctly scheduled.
This issue only happens with targets that can build multiple sub-architectures at the same time (ARM32, MIPS, PPC64) possibly causing the symlinking to run at the sametime. On Apple this includes ARM64 and x86-64, making this a bit easier to hit.

The add_custom_target() solution noted in the mailing list seems the most appropriate to force ordering but still keep parallelism as noted by @thakis. Let's go with that.

Jul 20 2021, 12:52 PM · Restricted Project
steven_wu updated the summary of D106305: [compiler-rt][CMake][arm64] Use a custom target for symlinking LSE sources.
Jul 20 2021, 9:12 AM · Restricted Project
steven_wu added a comment to D106305: [compiler-rt][CMake][arm64] Use a custom target for symlinking LSE sources.

The configured directory is rather large so let me give some more detail from the makefile generated. The custom target is generated in two of the makefiles I mentioned above, like following:

projects/compiler-rt/lib/builtins/outline_atomic_helpers.dir/outline_atomic_cas1_1.S:
        @$(CMAKE_COMMAND) -E cmake_echo_color --switch=$(COLOR) --blue --bold --progress-dir=/Users/steven/makefile_build/CMakeFiles --progress-num=$(CMAKE_PROGRESS_1) "Generating outline_atomic_helpers.dir/outline_atomic_cas1_1.S"
        cd /Users/steven/makefile_build/projects/compiler-rt/lib/builtins && /usr/local/bin/cmake -E create_symlink /Users/steven/llvm-project/compiler-rt/lib/builtins/aarch64/lse.S /Users/steven/makefile_build/projects/compiler-rt/lib/builtins/outline_atomic_helpers.dir/outline_atomic_cas1_1.S
Jul 20 2021, 9:10 AM · Restricted Project