User Details
- User Since
- Nov 4 2014, 3:46 PM (394 w, 2 d)
Yesterday
Thanks! LGTM.
Fri, May 20
Is there any issues currently if we just always use opaque pointer in LTO?
Thu, May 19
LGTM. Can you add a test for your usecase?
Tue, May 17
This breaks darwin bot: https://green.lab.llvm.org/green/job/clang-stage1-RA/29421/
Tue, May 3
Apr 20 2022
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 19 2022
Mar 30 2022
Mar 15 2022
Mar 14 2022
Add win32.
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()
This is actually fixed in f51d7e4bae9e861e711ad9711599456fc2f1bbca before this is reviewed. I will just close this.
This breaks implicit module on macOS bots: https://green.lab.llvm.org/green/job/lldb-cmake/42098/console
Mar 9 2022
Mar 3 2022
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 ~~~~~~~~^
Feb 8 2022
LGTM
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.
Jan 18 2022
Jan 17 2022
clang-format
Jan 14 2022
Rebase to TOT
Rebase the change to TOT
Jan 13 2022
Fix test name.
Address review feedback.
Rename the variables to be more accurate.
Dec 13 2021
Thanks! LGTM now.
Dec 10 2021
Dec 1 2021
@lhames ping
Nov 30 2021
Fix the rebase issue that mixed up with D114749
Address review feedback
Address review feedback
Nov 29 2021
LGTM.
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 19 2021
The change is fine. Can you elaborate a bit more about your use case? Or at least add a test for it?
Nov 15 2021
Update the patch to trust user's action and update symbols to fit in the new block
Address review feedback after talking with Lang and Duncan offline.
Oct 29 2021
Oct 27 2021
Oct 22 2021
LGTM. Can you use a better commit message?
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 21 2021
Oct 18 2021
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 7 2021
Sep 27 2021
Sep 23 2021
LGTM.
Sep 21 2021
+ @davide who might have more context of LC_CODE_SIGNITURE section
Sep 20 2021
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.
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.
LGTM.
Sep 9 2021
Sep 8 2021
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 7 2021
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.
Aug 30 2021
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.
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 16 2021
Aug 12 2021
LGTM. Thanks
Aug 9 2021
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?
Can you unify the duplicated code or just put the FIXME back saying need to fix the duplicated code in the future?
Aug 4 2021
That is said, I am fine with the clean up but this probably doesn't resolve FIXME.
I think the FIXME is about not duplicating code from LTOCodeGenerator, not how terrible if statement looks.
Jul 29 2021
Jul 27 2021
LGTM in general. Just two small comments inline. Thanks!
Comment inline.
Jul 26 2021
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".
Works now. Thanks!
Jul 25 2021
I still get the same error. 😿
Jul 23 2021
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 22 2021
The error says ios_arm64e because it duplicates for every single os arm64 combination. osx_arm64, osx_arm64e, ios_arm64, ios_arm64e, etc.
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 20 2021
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