[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
weliveindetail wrote: The above patch landed after `release/18.x` branched. It drops the check for `CMAKE_SYSTEM_PROCESSOR` as discussed in this thread and only relies on the linker-flag check. This seems to be the right thing to do. I checked on 32-bit Raspbian @ RPi4b: It correctly defaults to triple `arm-linux-gnueabihf`, but `CMAKE_SYSTEM_PROCESSOR` is `AARCH64`. Thus, on current `release/18.x` we skip adding the `--long-plt` flag in this case. (It doesn't have the resources to link clang-repl, but this is still incorrect.) It might be a backporting candidate. https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
weliveindetail wrote: I was wondering as well and checked the codebase for existing uses, but no findings. Yes, clang has no JIT and may not reach the limit. And yes, LLDB is mostly an `.so` where the linker's approach might differ and/or it's just not built frequently for ARM. Myself, I usually opt for remote debugging and just build lldb-server. https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
mstorsjo wrote: > > I don't really know more about the issue that requires --long-plt at the > > moment and why it's only needed for clang-repl > > clang-repl binary size is ~3.7G in debug mode and this seems to exceed the > branch range of default ARM PLT slots. The instruction sequence that's > necessary for long-range PLT slots is likely more expensive. I guess this > situation is too much of an edge-case to make the effort and detect it > upfront, so they just bail out if it happens an ask for the flag. Right, I see. Though - why is this only an issue for the clang-repl binary and not the other clang-based ones? Does it happen to hit some maximum when it uses both everything that is in normal clang, plus all the JIT stuff? (Normally I would expect LLDB to be the largest one, as it includes much of Clang, but that's also always split into a separate liblldb.so.) https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
weliveindetail wrote: > I don't really know more about the issue that requires --long-plt at the > moment and why it's only needed for clang-repl clang-repl binary size is ~3.7G in debug mode and this seems to exceed the branch range of default ARM PLT slots. The instruction sequence that's necessary for long-range PLT slots is likely more expensive. I guess this situation is too much of an edge-case to make the effort and detect it upfront, so they just bail out if it happens an ask for the flag. > If you'd want it to hit more universally, perhaps just remove the > architecture check - if we test that the linker does support `--long-plt` > without erroring out, we probably can add it, even if we don't really know > the exact architecture we're linking for? Yes, that sounds like the better option. We give it a try later today. https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
mstorsjo wrote: > Oh, I usually don't do that, but it's certainly a valid point. Can you think > of a better way to express the condition here? We need `-Wl,--long-plt` for > ARM targets whenever the used linker supports it. Otherwise we have to assume > that it emits such PLTs by default. Not sure; architecture detection in CMake generally is problematic. It's possible to infer the actual real destination architecture with a compiler test (like compiling and testing if `__arm__` is defined). CMake doesn't really provide anything out of the box for that though (it does provide `CMAKE_C_COMPILER_ARCHITECTURE_ID` on MSVC like compilers, but not elsewhere, see https://gitlab.kitware.com/cmake/cmake/-/issues/17702), and it feels like overkill to add such a compile test here. Also note that `CMAKE_SYSTEM_PROCESSOR` is unclear in Darwin universal builds anyway - if I'm compiling with `-DCMAKE_OSX_ARCHITECTURES=x86_64;arm64` what will be set in `CMAKE_SYSTEM_PROCESSOR`? :-) I think this current form of the check might be ok enough (I don't really know more about the issue that requires `--long-plt` at the moment and why it's only needed for clang-repl). If you'd want it to hit more universally, perhaps just remove the architecture check - if we test that the linker does support `--long-plt` without erroring out, we probably can add it, even if we don't really know the exact architecture we're linking for? https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
weliveindetail wrote: Oh, I usually don't do that, but it's certainly a valid point. Can you think of a better way to express the condition here? We need `-Wl,--long-plt` for ARM targets whenever the used linker supports it. Otherwise we have to assume that it emits such PLTs by default. https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
mstorsjo wrote: > > When cross compiling LLVM, I never have set `CMAKE_SYSTEM_PROCESSOR` so > > far, since we don't really have anything that uses it (before this), which > > means that this expands to an empty string. I guess I should set it still > > though. > > Yes, I am just getting used to it as well. I think it's worth noting that > this should be set in a toolchain file, because CMake seems to have special > handling for them. When I pass `-DCMAKE_SYSTEM_PROCESSOR=ARM` at > configuration time, it gets overwritten with my host arch. I think that's (somewhat?) expected. When you're not cross compiling, cmake explicitly sets `CMAKE_SYSTEM_PROCESSOR` to `CMAKE_HOST_SYSTEM_PROCESSOR`, ignoring whatever you set manually. (Dunno if it would make any difference if you'd set it in a toolchain file.) Only if you're cross compiling (which CMake defines as if you're setting `CMAKE_SYSTEM_NAME`, regardless if cross compiling from Linux to Linux on another arch), it doesn't set it and passes whatever you set originally through. This actually makes `CMAKE_SYSTEM_PROCESSOR` a bit problematic; if you're on x86_64 but targeting i386, or on aarch64 but targeting arm, you might not want to consider it a cross build (as you can execute the build products) but CMake then will hide whatever you're really targeting with the info gathered from the host environment. https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
weliveindetail wrote: CMake does always have more surprises to offer :) Thanks for fixing it right away @mstorsjo! > When cross compiling LLVM, I never have set `CMAKE_SYSTEM_PROCESSOR` so far, > since we don't really have anything that uses it (before this), which means > that this expands to an empty string. I guess I should set it still though. Yes, I am just getting used to it as well. I think it's worth noting that this should be set in a toolchain file, because CMake seems to have special handling for them. When I pass `-DCMAKE_SYSTEM_PROCESSOR=ARM` at configuration time, it gets overwritten with my host arch. https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
Stefan =?utf-8?q?Gr=C3=A4nitz?= Message-ID: In-Reply-To: shiltian wrote: > @shiltian Does that fix the issue for you? Yes, thanks! https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
weliveindetail wrote: @shiltian Does that fix the issue for you? https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
Stefan =?utf-8?q?Gränitz?= Message-ID: In-Reply-To: shiltian wrote: > Oh, that's unfortunate. Let me add a check for `LLVM_USE_LINKER=gold`. AFAIK > it's never used on macOS. It might be better to check linker flag (https://cmake.org/cmake/help/latest/module/CheckLinkerFlag.html). https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
weliveindetail wrote: Oh, that's unfortunate. Let me add a check for `LLVM_USE_LINKER=gold`. AFAIK it's never used on macOS. https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
Stefan =?utf-8?q?Gränitz?= Message-ID: In-Reply-To: shiltian wrote: This breaks the build on macOS because the linker doesn't support `--long-plt`. Please fix it or revert the patch. https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
https://github.com/weliveindetail closed https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
weliveindetail wrote: Thank for the quick review! Toolchain files seem to set a lowercase string sometimes, so I added a case-conversion. https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
https://github.com/weliveindetail updated https://github.com/llvm/llvm-project/pull/78959 From 0449f8fc14a703aae515db1696bbbee578914629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Sat, 20 Jan 2024 11:13:45 +0100 Subject: [PATCH 1/2] [clang-repl] Fix linker error on ARM Error in gold linker is: PLT offset too large, try linking with --long-plt --- clang/tools/clang-repl/CMakeLists.txt | 4 1 file changed, 4 insertions(+) diff --git a/clang/tools/clang-repl/CMakeLists.txt b/clang/tools/clang-repl/CMakeLists.txt index 2ccbe292fd49e09..8589e37418354e7 100644 --- a/clang/tools/clang-repl/CMakeLists.txt +++ b/clang/tools/clang-repl/CMakeLists.txt @@ -22,3 +22,7 @@ clang_target_link_libraries(clang-repl PRIVATE if(CLANG_PLUGIN_SUPPORT) export_executable_symbols_for_plugins(clang-repl) endif() + +if(${CMAKE_SYSTEM_PROCESSOR} MATCHES "ARM") + target_link_options(clang-repl PRIVATE LINKER:--long-plt) +endif() From f68b96c87cf165b4cf6d453b8f817f9a3353b3d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Mon, 22 Jan 2024 13:37:55 +0100 Subject: [PATCH 2/2] fixup! [clang-repl] Fix linker error on ARM Toolchain files sometimes set a lowercase string, like armv7 --- clang/tools/clang-repl/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/tools/clang-repl/CMakeLists.txt b/clang/tools/clang-repl/CMakeLists.txt index 8589e37418354e7..b0aaf39f0115472 100644 --- a/clang/tools/clang-repl/CMakeLists.txt +++ b/clang/tools/clang-repl/CMakeLists.txt @@ -23,6 +23,7 @@ if(CLANG_PLUGIN_SUPPORT) export_executable_symbols_for_plugins(clang-repl) endif() -if(${CMAKE_SYSTEM_PROCESSOR} MATCHES "ARM") +string(TOUPPER ${CMAKE_SYSTEM_PROCESSOR} system_processor) +if(${system_processor} MATCHES "ARM") target_link_options(clang-repl PRIVATE LINKER:--long-plt) endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
https://github.com/vgvassilev approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Stefan Gränitz (weliveindetail) Changes I cross-compile clang-repl with GCC-10 on Ubuntu 20.04 and get this error when linking with gold: PLT offset too large, try linking with --long-plt --- Full diff: https://github.com/llvm/llvm-project/pull/78959.diff 1 Files Affected: - (modified) clang/tools/clang-repl/CMakeLists.txt (+4) ``diff diff --git a/clang/tools/clang-repl/CMakeLists.txt b/clang/tools/clang-repl/CMakeLists.txt index 2ccbe292fd49e09..8589e37418354e7 100644 --- a/clang/tools/clang-repl/CMakeLists.txt +++ b/clang/tools/clang-repl/CMakeLists.txt @@ -22,3 +22,7 @@ clang_target_link_libraries(clang-repl PRIVATE if(CLANG_PLUGIN_SUPPORT) export_executable_symbols_for_plugins(clang-repl) endif() + +if(${CMAKE_SYSTEM_PROCESSOR} MATCHES "ARM") + target_link_options(clang-repl PRIVATE LINKER:--long-plt) +endif() `` https://github.com/llvm/llvm-project/pull/78959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)
https://github.com/weliveindetail created https://github.com/llvm/llvm-project/pull/78959 I cross-compile clang-repl with GCC-10 on Ubuntu 20.04 and get this error when linking with gold: PLT offset too large, try linking with --long-plt From 0449f8fc14a703aae515db1696bbbee578914629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= Date: Sat, 20 Jan 2024 11:13:45 +0100 Subject: [PATCH] [clang-repl] Fix linker error on ARM Error in gold linker is: PLT offset too large, try linking with --long-plt --- clang/tools/clang-repl/CMakeLists.txt | 4 1 file changed, 4 insertions(+) diff --git a/clang/tools/clang-repl/CMakeLists.txt b/clang/tools/clang-repl/CMakeLists.txt index 2ccbe292fd49e09..8589e37418354e7 100644 --- a/clang/tools/clang-repl/CMakeLists.txt +++ b/clang/tools/clang-repl/CMakeLists.txt @@ -22,3 +22,7 @@ clang_target_link_libraries(clang-repl PRIVATE if(CLANG_PLUGIN_SUPPORT) export_executable_symbols_for_plugins(clang-repl) endif() + +if(${CMAKE_SYSTEM_PROCESSOR} MATCHES "ARM") + target_link_options(clang-repl PRIVATE LINKER:--long-plt) +endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits