[clang] [clang-repl] Fix PLT offset too large linker error on ARM (PR #78959)

2024-01-24 Thread Stefan Gränitz via cfe-commits

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)

2024-01-23 Thread Stefan Gränitz via cfe-commits

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)

2024-01-23 Thread Martin Storsjö via cfe-commits

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)

2024-01-23 Thread Stefan Gränitz via cfe-commits

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)

2024-01-23 Thread Martin Storsjö via cfe-commits

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)

2024-01-23 Thread Stefan Gränitz via cfe-commits

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)

2024-01-23 Thread Martin Storsjö via cfe-commits

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)

2024-01-23 Thread Stefan Gränitz via cfe-commits

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)

2024-01-22 Thread Shilei Tian via cfe-commits
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)

2024-01-22 Thread Stefan Gränitz via cfe-commits

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)

2024-01-22 Thread Shilei Tian via cfe-commits
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)

2024-01-22 Thread Stefan Gränitz via cfe-commits

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)

2024-01-22 Thread Shilei Tian via cfe-commits
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)

2024-01-22 Thread Stefan Gränitz via cfe-commits

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)

2024-01-22 Thread Stefan Gränitz via cfe-commits

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)

2024-01-22 Thread Stefan Gränitz via cfe-commits

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)

2024-01-22 Thread Vassil Vassilev via cfe-commits

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)

2024-01-22 Thread via cfe-commits

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)

2024-01-22 Thread Stefan Gränitz via cfe-commits

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