This is an archive of the discontinued LLVM Phabricator instance.

Differential D120019

Replace the use of TARGET_TRIPLE
ClosedPublic

Authored by phosek on Feb 17 2022, 1:23 AM.

Details

Reviewers
rovka
gkistanova
Commits
rZORGe3ae5eb67e93: Replace the use of TARGET_TRIPLE
Summary

This is an internal variable that's being renamed to LLVM_TARGET_TRIPLE
in D119918, instead use LLVM_DEFAULT_TARGET_TRIPLE.

Diff Detail

Event Timeline

phosek requested review of this revision.Feb 17 2022, 1:23 AM
phosek created this revision.
rovka accepted this revision.Feb 17 2022, 4:42 AM

LGTM, thanks!

This revision is now accepted and ready to land.Feb 17 2022, 4:42 AM
Closed by commit rZORGe3ae5eb67e93: Replace the use of TARGET_TRIPLE (authored by phosek). · Explain WhyMar 4 2022, 12:50 PM
This revision was automatically updated to reflect the committed changes.
phosek added a commit: rZORGe3ae5eb67e93: Replace the use of TARGET_TRIPLE.
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2022, 12:50 PM
vvereschaka added a subscriber: vvereschaka.Mar 4 2022, 1:38 PM

Hi @phosek,

the TARGET_TRIPLE is a parameter for the CrossWinToARMLinux.cmake cache file and used within it to configure the build, including LLVM_DEFAULT_TARGET_TRIPLE. Your changes will break the Win-to-Linux cross toolchain builds.
Would you revert your commit?

vvereschaka added a comment.Mar 4 2022, 1:42 PM

Those builders are currently on staging buildbot.

https://lab.llvm.org/staging/#/builders/36
https://lab.llvm.org/staging/#/builders/41

Aarch64 will be moved to the regular buildbot soon.
Armv7 got some problems (crash for some libc++ test) and it still under investigation.

phosek added a comment.Mar 4 2022, 2:08 PM
In D120019#3361008, @vvereschaka wrote:

Those builders are currently on staging buildbot.

https://lab.llvm.org/staging/#/builders/36
https://lab.llvm.org/staging/#/builders/41

Aarch64 will be moved to the regular buildbot soon.
Armv7 got some problems (crash for some libc++ test) and it still under investigation.

I was planning on landing https://reviews.llvm.org/D119918 which renames TARGET_TRIPLE in CrossWinToARMLinux.cmake, would that address the issue?

phosek added a reverting change: rZORG42cd13996838: Revert "Replace the use of TARGET_TRIPLE".Mar 4 2022, 2:18 PM
vvereschaka added a comment.Mar 4 2022, 2:31 PM

I was planning on landing https://reviews.llvm.org/D119918 which renames TARGET_TRIPLE in CrossWinToARMLinux.cmake, would that address the issue?

This differential uses the old version of CrossWinToARMLinux.cmake and this cache file got a lot of changes currently. These changes will break merging of your changes.

Also, TARGET_TRIPLE is using only within the cache file. It should not interfere with other your changes. Or you want to wipe out any allusion to TARGET_RIPLE within the source tree?

phosek added a comment.Mar 4 2022, 2:38 PM
In D120019#3361117, @vvereschaka wrote:

I was planning on landing https://reviews.llvm.org/D119918 which renames TARGET_TRIPLE in CrossWinToARMLinux.cmake, would that address the issue?

This differential uses the old version of CrossWinToARMLinux.cmake and this cache file got a lot of changes currently. These changes will break merging of your changes.

Also, TARGET_TRIPLE is using only within the cache file. It should not interfere with other your changes. Or you want to wipe out any allusion to TARGET_RIPLE within the source tree?

I'm fine for CrossWinToARMLinux.cmake to continue using TARGET_TRIPLE to make it explicit that it's *not* the same variable as LLVM_TARGET_TRIPLE and avoid similar mistakes as I did in the future. I have reverted this change and updated D119918 and excluded CrossWinToARMLinux.cmake.

vvereschaka added a comment.Mar 4 2022, 3:10 PM

I'm testing https://reviews.llvm.org/D119918 locally on the builders. Looks like TARGET_TRIPLE as a cache file parameter slightly affects the configuration. I'm getting the warnings during the initial configuration.
I'll replace the parameter name to TOOLCHAIN_TARGET_TRIPLE to avoid the possible future mistakes and just in case.

I'll update the cache file and the buildbot configuration.

DoyleLi added a reverting change: D158142: Improved InformativeMailNotifier..Aug 16 2023, 10:20 PM

Revision Contents

PathSize
buildbot/
osuosl/
master/
config/
5 lines
4 lines

Diff 413111

buildbot/osuosl/master/config/builders.py

Show First 20 Lines • Show All 175 Lines • ▼ Show 20 Lines 'factory' : XToolchainBuilder.getCmakeWithMSVCBuildFactory(
("libc++", ("libc++",
['python', 'bin/llvm-lit.py', ['python', 'bin/llvm-lit.py',
'-v', '-vv', '--threads=32', '-v', '-vv', '--threads=32',
'runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libcxx/test', 'runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libcxx/test',
]) ])
], ],
extra_configure_args=[ extra_configure_args=[
"-DLLVM_TARGETS_TO_BUILD=ARM", "-DLLVM_TARGETS_TO_BUILD=ARM",
"-DTARGET_TRIPLE=armv7-unknown-linux-gnueabihf", "-DLLVM_DEFAULT_TARGET_TRIPLE=armv7-unknown-linux-gnueabihf",
"-DDEFAULT_SYSROOT=C:/buildbot/.arm-ubuntu", "-DDEFAULT_SYSROOT=C:/buildbot/.arm-ubuntu",
"-DZLIB_ROOT=C:/buildbot/.zlib-win32", "-DZLIB_ROOT=C:/buildbot/.zlib-win32",
"-DLLVM_LIT_ARGS=-v -vv --threads=32", "-DLLVM_LIT_ARGS=-v -vv --threads=32",
WithProperties("%(remote_test_host:+-DREMOTE_TEST_HOST=)s%(remote_test_host:-)s"), WithProperties("%(remote_test_host:+-DREMOTE_TEST_HOST=)s%(remote_test_host:-)s"),
WithProperties("%(remote_test_user:+-DREMOTE_TEST_USER=)s%(remote_test_user:-)s"), WithProperties("%(remote_test_user:+-DREMOTE_TEST_USER=)s%(remote_test_user:-)s"),
], ],
cmake_cache="../llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake")}, cmake_cache="../llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake")},
Show All 22 Lines 'factory' : XToolchainBuilder.getCmakeWithMSVCBuildFactory(
("libc++", ("libc++",
['python', 'bin/llvm-lit.py', ['python', 'bin/llvm-lit.py',
'-v', '-vv', '--threads=32', '-v', '-vv', '--threads=32',
'runtimes/runtimes-aarch64-unknown-linux-gnu-bins/libcxx/test', 'runtimes/runtimes-aarch64-unknown-linux-gnu-bins/libcxx/test',
]) ])
], ],
extra_configure_args=[ extra_configure_args=[
"-DLLVM_TARGETS_TO_BUILD=AArch64", "-DLLVM_TARGETS_TO_BUILD=AArch64",
"-DTARGET_TRIPLE=aarch64-unknown-linux-gnu", "-DLLVM_DEFAULT_TARGET_TRIPLE=aarch64-unknown-linux-gnu",
"-DDEFAULT_SYSROOT=C:/buildbot/.aarch64-ubuntu", "-DDEFAULT_SYSROOT=C:/buildbot/.aarch64-ubuntu",
"-DZLIB_ROOT=C:/buildbot/.zlib-win32", "-DZLIB_ROOT=C:/buildbot/.zlib-win32",
"-DLLVM_LIT_ARGS=-v -vv --threads=32", "-DLLVM_LIT_ARGS=-v -vv --threads=32",
WithProperties("%(remote_test_host:+-DREMOTE_TEST_HOST=)s%(remote_test_host:-)s"), WithProperties("%(remote_test_host:+-DREMOTE_TEST_HOST=)s%(remote_test_host:-)s"),
WithProperties("%(remote_test_user:+-DREMOTE_TEST_USER=)s%(remote_test_user:-)s"), WithProperties("%(remote_test_user:+-DREMOTE_TEST_USER=)s%(remote_test_user:-)s"),
], ],
cmake_cache="../llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake")}, cmake_cache="../llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake")},
▲ Show 20 Lines • Show All 523 Lines • ▼ Show 20 Lines 'factory' : ClangBuilder.getClangCMakeBuildFactory(
jobs=16, jobs=16,
checkout_clang_tools_extra=False, checkout_clang_tools_extra=False,
checkout_compiler_rt=False, checkout_compiler_rt=False,
checkout_lld=False, checkout_lld=False,
env={'LD_LIBRARY_PATH': '/local/clang+llvm-12.0.0-x86_64-linux-gnu-ubuntu-16.04/lib'}, env={'LD_LIBRARY_PATH': '/local/clang+llvm-12.0.0-x86_64-linux-gnu-ubuntu-16.04/lib'},
extra_cmake_args=[ extra_cmake_args=[
"-DCMAKE_BUILD_TYPE:STRING=Release", "-DCMAKE_BUILD_TYPE:STRING=Release",
"-DLLVM_TARGETS_TO_BUILD:STRING=Hexagon", "-DLLVM_TARGETS_TO_BUILD:STRING=Hexagon",
"-DTARGET_TRIPLE:STRING=hexagon-unknown-elf",
"-DLLVM_DEFAULT_TARGET_TRIPLE:STRING=hexagon-unknown-elf", "-DLLVM_DEFAULT_TARGET_TRIPLE:STRING=hexagon-unknown-elf",
"-DLLVM_TARGET_ARCH:STRING=hexagon-unknown-elf", "-DLLVM_TARGET_ARCH:STRING=hexagon-unknown-elf",
"-DLLVM_BUILD_RUNTIME:BOOL=OFF", "-DLLVM_BUILD_RUNTIME:BOOL=OFF",
"-DCMAKE_VERBOSE_MAKEFILE:BOOL=ON", "-DCMAKE_VERBOSE_MAKEFILE:BOOL=ON",
"-DLLVM_ENABLE_PIC:BOOL=ON", "-DLLVM_ENABLE_PIC:BOOL=ON",
"-DLLVM_ENABLE_ASSERTIONS:BOOL=ON", "-DLLVM_ENABLE_ASSERTIONS:BOOL=ON",
"-DLLVM_INCLUDE_TOOLS:BOOL=ON", "-DLLVM_INCLUDE_TOOLS:BOOL=ON",
"-DLLVM_LIT_ARGS:STRING=-v", "-DLLVM_LIT_ARGS:STRING=-v",
▲ Show 20 Lines • Show All 1,547 Lines • Show Last 20 Lines

buildbot/osuosl/master/config/release_builders.py

Show First 20 Lines • Show All 120 Lines • ▼ Show 20 Lines 'factory' : XToolchainBuilder.getCmakeWithMSVCBuildFactory(
("libc++", ("libc++",
['python', 'bin/llvm-lit.py', ['python', 'bin/llvm-lit.py',
'-v', '-vv', '--threads=32', '-v', '-vv', '--threads=32',
'runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libcxx/test', 'runtimes/runtimes-armv7-unknown-linux-gnueabihf-bins/libcxx/test',
]) ])
], ],
extra_configure_args=[ extra_configure_args=[
"-DLLVM_TARGETS_TO_BUILD=ARM", "-DLLVM_TARGETS_TO_BUILD=ARM",
"-DTARGET_TRIPLE=armv7-unknown-linux-gnueabihf", "-DLLVM_DEFAULT_TARGET_TRIPLE=armv7-unknown-linux-gnueabihf",
"-DDEFAULT_SYSROOT=C:/buildbot/.arm-ubuntu", "-DDEFAULT_SYSROOT=C:/buildbot/.arm-ubuntu",
"-DZLIB_ROOT=C:/buildbot/.zlib-win32", "-DZLIB_ROOT=C:/buildbot/.zlib-win32",
"-DLLVM_LIT_ARGS=-v -vv --threads=32", "-DLLVM_LIT_ARGS=-v -vv --threads=32",
WithProperties("%(remote_test_host:+-DREMOTE_TEST_HOST=)s%(remote_test_host:-)s"), WithProperties("%(remote_test_host:+-DREMOTE_TEST_HOST=)s%(remote_test_host:-)s"),
WithProperties("%(remote_test_user:+-DREMOTE_TEST_USER=)s%(remote_test_user:-)s"), WithProperties("%(remote_test_user:+-DREMOTE_TEST_USER=)s%(remote_test_user:-)s"),
], ],
cmake_cache="../llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake")}, cmake_cache="../llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake")},
Show All 22 Lines 'factory' : XToolchainBuilder.getCmakeWithMSVCBuildFactory(
("libc++", ("libc++",
['python', 'bin/llvm-lit.py', ['python', 'bin/llvm-lit.py',
'-v', '-vv', '--threads=32', '-v', '-vv', '--threads=32',
'runtimes/runtimes-aarch64-unknown-linux-gnu-bins/libcxx/test', 'runtimes/runtimes-aarch64-unknown-linux-gnu-bins/libcxx/test',
]) ])
], ],
extra_configure_args=[ extra_configure_args=[
"-DLLVM_TARGETS_TO_BUILD=AArch64", "-DLLVM_TARGETS_TO_BUILD=AArch64",
"-DTARGET_TRIPLE=aarch64-unknown-linux-gnu", "-DLLVM_DEFAULT_TARGET_TRIPLE=aarch64-unknown-linux-gnu",
"-DDEFAULT_SYSROOT=C:/buildbot/.aarch64-ubuntu", "-DDEFAULT_SYSROOT=C:/buildbot/.aarch64-ubuntu",
"-DZLIB_ROOT=C:/buildbot/.zlib-win32", "-DZLIB_ROOT=C:/buildbot/.zlib-win32",
"-DLLVM_LIT_ARGS=-v -vv --threads=32", "-DLLVM_LIT_ARGS=-v -vv --threads=32",
WithProperties("%(remote_test_host:+-DREMOTE_TEST_HOST=)s%(remote_test_host:-)s"), WithProperties("%(remote_test_host:+-DREMOTE_TEST_HOST=)s%(remote_test_host:-)s"),
WithProperties("%(remote_test_user:+-DREMOTE_TEST_USER=)s%(remote_test_user:-)s"), WithProperties("%(remote_test_user:+-DREMOTE_TEST_USER=)s%(remote_test_user:-)s"),
], ],
cmake_cache="../llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake")}, cmake_cache="../llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake")},
▲ Show 20 Lines • Show All 53 Lines • Show Last 20 Lines