Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use llvm 13+'s [[musttail]] for dynapi #9229

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

madebr
Copy link
Contributor

@madebr madebr commented Mar 9, 2024

Suggested here as an attempt to reduce file sizes (#9206).

When building a 32-bit msys2 llvm15 toolchain, file size reductions are 10 kiB to nothing (a stripped executable is around 1.88MB)

  • Because llvm's [[musttail]] requires an explicit return, this pr introduces SDL_DYNAPI_PROC for functions returning something and SDL_DYNAPI_PROC_VOID for functions returning nothing. It also introduces SDL_DYNAPI_PROC_NO_TAILCALL for explicitly disabling [[musttail]] (see below why). gendynapi.py has been modified to support these.
  • Introduce __attribute__ ((musttail)) in appropriate locations in SDL_dynapi.c.

[[musttail]] has been explicitly disabled for functions accepting a SDL_GUID argument by value. Testautomation was segfaulting. I'm not entirely sure why this happens.
Ironically, disabling [[musttail]] (for DSL_GUIDToString) produces less code that works and code that uses a tail call.

SDL_GUIDToString, built with llvm's [[tailcall]] (segfaults)

1001bc80 <_SDL_GUIDToString>:
1001bc80: 8b 4c 24 18                   mov     ecx, dword ptr [esp + 0x18]
1001bc84: 8b 54 24 14                   mov     edx, dword ptr [esp + 0x14]
1001bc88: a1 e0 92 1b 10                mov     eax, dword ptr [0x101b92e0]
1001bc8d: 0f 10 44 24 04                movups  xmm0, xmmword ptr [esp + 0x4]
1001bc92: 0f 11 04 24                   movups  xmmword ptr [esp], xmm0
1001bc96: 0f 11 44 24 04                movups  xmmword ptr [esp + 0x4], xmm0
1001bc9b: 89 54 24 14                   mov     dword ptr [esp + 0x14], edx
1001bc9f: 89 4c 24 18                   mov     dword ptr [esp + 0x18], ecx
1001bca3: ff e0                         jmp     eax
1001bca5: 66 66 2e 0f 1f 84 00 00 00 00 00      nop     word ptr cs:[eax + eax]

SDL_GUIDToString, build without llvm's [[tailcall]] (works)

1001bc80 <_SDL_GUIDToString>:
1001bc80: ff 25 e0 92 1b 10             jmp     dword ptr [0x101b92e0]
1001bc86: 66 2e 0f 1f 84 00 00 00 00 00 nop     word ptr cs:[eax + eax]

FYI, llvm generates code that is peculiar:

1001bcb0 <_SDL_GamepadConnected>:
1001bcb0: 8b 44 24 04                   mov     eax, dword ptr [esp + 0x4]
1001bcb4: 89 44 24 04                   mov     dword ptr [esp + 0x4], eax
1001bcb8: ff 25 e4 92 1b 10             jmp     dword ptr [0x101b92e4]

Does anybody know why this happens? The mov's can be removed, can't they?
I observe the same behavior in other SDL dynapi entry functions accepting an argument, and it is worse for functions accepting multiple arguments.
e.g.

1001c5c0 <_SDL_GetMasksForPixelFormatEnum>:
1001c5c0: 0f 10 44 24 0c                movups  xmm0, xmmword ptr [esp + 0xc]
1001c5c5: 8b 44 24 08                   mov     eax, dword ptr [esp + 0x8]
1001c5c9: 8b 4c 24 04                   mov     ecx, dword ptr [esp + 0x4]
1001c5cd: 89 4c 24 04                   mov     dword ptr [esp + 0x4], ecx
1001c5d1: 89 44 24 08                   mov     dword ptr [esp + 0x8], eax
1001c5d5: 0f 11 44 24 0c                movups  xmmword ptr [esp + 0xc], xmm0
1001c5da: ff 25 94 94 1b 10             jmp     dword ptr [0x101b9494]

Description

Existing Issue(s)

This allows a follow-up macro to use "return" in void functions,
which is required when using llvm's [[musttail]].
Tail calls are disabled for functions acception a SDL_GUID argument
by value.
@madebr
Copy link
Contributor Author

madebr commented Mar 9, 2024

The SDL_GUID issue might not be detected because of the lack of a warning:
llvm/llvm-project#56676

llvm/llvm-project#72390 is also what happens (I think)

@madebr
Copy link
Contributor Author

madebr commented Mar 9, 2024

I don't see the weird x86 code on Linux, when building with a (slightly) more modern llvm toolchain.

@okuoku
Copy link
Contributor

okuoku commented Mar 10, 2024

musttail sorta premature on older Clang e.g.) llvm/llvm-project#50758 so it'd be safer to add some escape hatch that ignores HAVE_ATTRIBUTE_MUSTTAIL (or just filter out older clang there)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants