Remove iOS 5 check for tailcalls on ARM

[Un1q32]

Fixes #102053

The check was added in 8decdc4, and at the time iOS 5 was the latest iOS version, before that commit tail calls were disabled for all ARMv7 targets. Testing a build of wasm3 with the patch on a device running iOS 3.0 shows a noticeable performance improvement and no issues.

[llvmbot]

@llvm/pr-subscribers-backend-arm

Author: None (Un1q32)

Changes

Fixes #102053

The check was added in 8decdc4, and at the time iOS 5 was the latest iOS version. Testing a build of wasm3 with the patch on a device running iOS 3.0 shows a noticeable performance improvement and no issues.

---

Full diff: https://github.com/llvm/llvm-project/pull/133354.diff

1 Files Affected:

• (modified) llvm/lib/Target/ARM/ARMSubtarget.cpp (-3)

diff --git a/llvm/lib/Target/ARM/ARMSubtarget.cpp b/llvm/lib/Target/ARM/ARMSubtarget.cpp
index 893084785e6f0..759070c6f08da 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.cpp
+++ b/llvm/lib/Target/ARM/ARMSubtarget.cpp
@@ -226,9 +226,6 @@ void ARMSubtarget::initSubtargetFeatures(StringRef CPU, StringRef FS) {
 
   SupportsTailCall = !isThumb1Only() || hasV8MBaselineOps();
 
-  if (isTargetMachO() && isTargetIOS() && getTargetTriple().isOSVersionLT(5, 0))
-    SupportsTailCall = false;
-
   switch (IT) {
   case DefaultIT:
     RestrictIT = false;

[Un1q32]

Not sure what those failed tests are testing, or how I would fix them.

[davemgreen]

They all run with -mtriple=armv7-apple-ios3.0 or -mtriple=armv7-apple-ios, and are presumably now tail-folding. You will need to update the CHECK lines in the tests to match the new output.

[Un1q32]

Tests fixed

[davemgreen]

Thanks, I've added some reviewers who might be able to say this is OK. As far as I understand it was likely disabled for older ios versions due to an issue in the linker, which likely isn't present any more?

[Un1q32]

As far as I understand it was likely disabled for older ios versions due to an issue in the linker, which likely isn't present any more?

If that's the case, I might try to find what linker versions have this issue, and then check the linker version to disable tail calls.

[jroelofs]

A quick dig through the relevant radars suggests that the issue was lack of support in the dynamic linker prior to iOS 5. This is the correct version check in that context. I think the proper fix for #102053 is to add a diagnostic in clang for this case.

[davemgreen]

I got that from 8decdc4 and might be wrong about the reason.

  /// SupportsTailCall - True if the OS supports tail call. The dynamic linker
  /// must be able to synthesize call stubs for interworking between ARM and
  /// Thumb.

[davemgreen]

A quick dig through the relevant radars suggests that the issue was lack of support in the dynamic linker prior to iOS 5. This is the correct version check in that context. I think the proper fix for #102053 is to add a diagnostic in clang for this case.

Oh I see.

[Un1q32]

I did a quick test with dyld and it seems to work. Just a main function that tail calls to a function in a dylib.

test.dylib:

int func(void) {
    return 42;
}

a.out:

extern int func(void);

int main(void) {
    return func();
}

I tried a thumb dylib and arm a.out, and an arm dylib and thumb a.out and both worked. Maybe the person making the commit didn't devices running iOS versions <5 to test on, and they added the 5.0 check just to be safe.

Also, no publicly released armv6 iOS device supported thumb instructions, so we could get away with enabling tailcalls on armv6 but not armv7 if there is an issue in dyld with arm and thumb.

[jroelofs]

A quick dig through the relevant radars suggests that the issue was lack of support in the dynamic linker prior to iOS 5. This is the correct version check in that context. I think the proper fix for #102053 is to add a diagnostic in clang for this case.

Oh I see.

Wait, sorry, I misspoke. It was the static linker at the time that didn't support it, and support landed in ld64-124.1 per rdar://9116044.

Maybe the person making the commit didn't devices running iOS versions <5 to test on, and they added the 5.0 check just to be safe.

Not based on my readings of the related radars, but if it works for you, go for it. The last devices that even supported iOS 4 are well past their support horizon.

[jroelofs]

LGTM

[Un1q32]

Wait, sorry, I misspoke. It was the static linker at the time that didn't support it, and support landed in ld64-124.1 per rdar://9116044.

Maybe I should change the check to make sure the linker version is at least 124.1 instead of removing the check entirely then. It's unlikely that many people will be using a linker version that old anymore, but since it would be so easy there's not much reason not to.

[Un1q32]

A quick look through LLVM's code shows the only logic to get the linker version for Darwin targets is in Clang, porting it over to LLVM would require more knowledge of the codebase than I currently have. Maybe another time.

[jroelofs]

Perfect case for a backend flag controlled by some frontend logic.

[rnk]

If someone is using modern LLVM to target iOS 3-4, is it reasonable to assume that they are also using a more modern linker that supports the necessary static relocations?

[Un1q32]

I am. I'm not that involved with the rest of the legacy iOS dev community anymore but last I remember they were mostly either using linux toolchains with modern LLVM and a modern LD64 port, or very rarely using old Xcode versions on old macOS VMs or actual old macs, using the Xcode provided compiler. One guy was using an old Linux environment to run the original Linux Darwin toolchain developed by Saurik in 2007-2008. I don't think anyone is using super old linkers with modern LLVM. https://github.com/tpoechtrager/cctools-port doesn't even have a port of LD64 that old, so you would have to be using old Xcode on old macOS. IIRC macports modern LLVM builds don't even work on macOS versions old enough to run a version of Xcode old enough to have that linker version. I've personally been able to build modern LLVM for as old as Leopard with very light patching, so its definitely possible but my project only has like 2 users that I know of and it also has a port of LD64 762 that the Clang package depends on anyway so I think it's safe to assume that there will be zero people negatively affected by this being merged.