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

[SR-7242] Usage of global constants to C functions generate suboptimal code (unless manually @convention(c)'d) #49790

Closed
weissi opened this issue Mar 20, 2018 · 4 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself performance

Comments

@weissi
Copy link
Member

weissi commented Mar 20, 2018

Previous ID SR-7242
Radar rdar://problem/38913070
Original Reporter @weissi
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, Performance
Assignee None
Priority Medium

md5: d1766f8972c3d593ae26d61989363305

Issue Description:

For this code

import Darwin

private let conventionCGlobal : @convention(c) (CInt, UnsafeRawPointer?, size_t) -> Int = write
private let normalGlobal: (CInt, UnsafeRawPointer?, size_t) -> Int = write

public enum System {
    @inline(never)
    public static func writeViaConventionCGlobal(fd: CInt, buffer: UnsafeRawPointer, size: size_t) -> ssize_t {
        return conventionCGlobal(fd, buffer, size)
    }

    @inline(never)
    public static func writeViaNormalGlobal(fd: CInt, buffer: UnsafeRawPointer, size: size_t) -> ssize_t {
        return normalGlobal(fd, buffer, size)
    }

    @inline(never)
    public static func writeViaModule(fd: CInt, buffer: UnsafeRawPointer, size: size_t) -> ssize_t {
        return Darwin.write(fd, buffer, size)
    }
}

I would expect the code generated for System.writeViaConventionCGlobal, System. writeViaNormalGlobal and System.writeViaModule to be identical. However, it's totally not:

This is the command I run

xcrun -toolchain org.swift.4120180319a swiftc -O -o test test.swift

which is

$ xcrun -toolchain org.swift.4120180319a swift --version
Apple Swift version 4.1-dev (LLVM 260a172ffb, Clang cd84be6c42, Swift 04baf31321)
Target: x86_64-apple-darwin17.4.0

(today's 4.1 snapshot).

This is the assembly that comes out of this

xcrun -toolchain org.swift.4120180319a swiftc -O -o test test.swift && otool -tV test | /Library/Developer/Toolchains/swift-4.1-DEVELOPMENT-SNAPSHOT-2018-03-19-a.xctoolchain/usr/bin/swift demangle

First System.writeViaModule

_static test.System.writeViaModule(fd: Swift.Int32, buffer: Swift.UnsafeRawPointer, size: Swift.Int) -> Swift.Int:
0000000100000d40        pushq   %rbp
0000000100000d41        movq    %rsp, %rbp
0000000100000d44        popq    %rbp
0000000100000d45        jmp     0x100000f02 ## symbol stub for: _write
0000000100000d4a        nopw    (%rax,%rax)

which is perfect. Then System.writeViaConventionCGlobal

_static test.System.writeViaConventionCGlobal(fd: Swift.Int32, buffer: Swift.UnsafeRawPointer, size: Swift.Int) -> Swift.Int:
0000000100000cb0        pushq   %rbp
0000000100000cb1        movq    %rsp, %rbp
0000000100000cb4        movq    _test.(conventionCGlobal in _83378C430F65473055F1BD53F3ADCDB7) : @convention(c) (Swift.Int32, Swift.UnsafeRawPointer?, Swift.Int) -> Swift.Int(%rip), %rax
0000000100000cbb        popq    %rbp
0000000100000cbc        jmpq    *%rax
0000000100000cbe        nop

still almost perfect (but why isn't the constant propagated). But then System.writeViaNormalGlobal

_static test.System.writeViaNormalGlobal(fd: Swift.Int32, buffer: Swift.UnsafeRawPointer, size: Swift.Int) -> Swift.Int:
0000000100000cc0        pushq   %rbp
0000000100000cc1        movq    %rsp, %rbp
0000000100000cc4        pushq   %r15
0000000100000cc6        pushq   %r14
0000000100000cc8        pushq   %r13
0000000100000cca        pushq   %r12
0000000100000ccc        pushq   %rbx
0000000100000ccd        pushq   %rax
0000000100000cce        movq    %rdx, %r13
0000000100000cd1        movq    %rsi, %r15
0000000100000cd4        movl    %edi, %r12d
0000000100000cd7        movq    _test.(normalGlobal in _83378C430F65473055F1BD53F3ADCDB7) : (Swift.Int32, Swift.UnsafeRawPointer?, Swift.Int) -> Swift.Int(%rip), %r14
0000000100000cde        movq    0x42b(%rip), %rbx
0000000100000ce5        movq    %rbx, %rdi
0000000100000ce8        callq   _swift_rt_swift_retain
0000000100000ced        movl    %r12d, %edi
0000000100000cf0        movq    %r15, %rsi
0000000100000cf3        movq    %r13, %rdx
0000000100000cf6        movq    %rbx, %r13
0000000100000cf9        callq   *%r14
0000000100000cfc        movq    %rax, %r14
0000000100000cff        movq    %rbx, %rdi
0000000100000d02        callq   _swift_rt_swift_release
0000000100000d07        movq    %r14, %rax
0000000100000d0a        addq    $0x8, %rsp
0000000100000d0e        popq    %rbx
0000000100000d0f        popq    %r12
0000000100000d11        popq    %r13
0000000100000d13        popq    %r14
0000000100000d15        popq    %r15
0000000100000d17        popq    %rbp
0000000100000d18        retq
0000000100000d19        nopl    (%rax)

which is pretty terrible. It'll affect SwiftNIO ( https://github.com/apple/swift-nio/blob/master/Sources/NIO/System.swift#L34-L60 ) but I guess we can work around.

Unfortunately also just writing

let sysWrite = write

gets you the suboptimal code, so the @convention(c) isn't inferred.

@belkadan
Copy link
Contributor

This wouldn't be something we can do across file/module boundaries, but it seems like a reasonable optimization within a file/module (depending on WMO). We could also say that you don't need to retain/release values loaded from a global/static let.

cc @eeckstein

@weissi
Copy link
Member Author

weissi commented Mar 27, 2018

@swift-ci create

@eeckstein
Copy link
Member

Sorry that we didn't deal with this earlier. Anyway, I tried with a recent compiler and the code looks optimal for all versions of the function.

@weissi
Copy link
Member Author

weissi commented Feb 14, 2020

@eeckstein awesome, thank you. I filed apple/swift-nio#1397 for us to undo hacks (we're being nice @gottesmm/@jckarter 🙂 )

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself performance
Projects
None yet
Development

No branches or pull requests

3 participants