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.