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-14100] Swift doesn't quite understand that optimising out whole Arrays is fine #56486

Closed
weissi opened this issue Jan 25, 2021 · 4 comments
Closed
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior.

Comments

@weissi
Copy link
Member

weissi commented Jan 25, 2021

Previous ID SR-14100
Radar rdar://problem/73569282
Original Reporter @weissi
Type Bug
Status Resolved
Resolution Done
Environment
swiftc -version
docker.io/swiftlang/swift:nightly-main-bionic
Swift version 5.4-dev (LLVM 7459512566cf7f4, Swift 8b5869ce6217b1f)
Target: x86_64-unknown-linux-gnu
Additional Detail from JIRA
Votes 0
Component/s
Labels Bug
Assignee None
Priority Medium

md5: 24d6e67a34f3fdc16dabfde09f778d00

Issue Description:

Consider this code:

@inline(never)
func doIt() -> Int {
    var sum = 0
    for x in [(1, "hello"),
              (2, "hello"),
              (3, "hello"),
              (4, "hello"),
              (5, "hello"),
              (6, "hello"),
              ] {
        sum += x.0
    }
    return sum
}

print(doIt())

Clearly, this code just prints 21 (1 + 2 + 3 + 4 + 5 + 6). For correctness, it doesn't actually need to build the whole array.

The compiler agrees with me: The end of the doIt function in assembly is just

    movl    $21, %eax
    addq    $176, %rsp
    popq    %rbx
    retq

which is cool. It literally puts a 21 into EAX. Good job.

Alas, it spits out a bunch more code that doesn't do anything:

swiftc -emit-assembly -O test.swift  | swift demangle | grep -A60 ^test.doIt
test.doIt() -> Swift.Int:
    pushq   %rbx
    subq    $176, %rsp
    leaq    (demangling cache variable for type metadata for Swift._ContiguousArrayStorage<(Swift.Int, Swift.String)>)(%rip), %rdi
    callq   __swift_instantiateConcreteTypeFromMangledName
    movq    %rsp, %rsi
    movq    %rax, %rdi
    callq   swift_initStackObject@PLT
    movaps  .LCPI2_0(%rip), %xmm0
    movups  %xmm0, 16(%rax)
    leaq    32(%rax), %rbx
    movaps  .LCPI2_1(%rip), %xmm0
    movups  %xmm0, 32(%rax)
    movabsq $-1945555039024054272, %rcx
    movq    %rcx, 48(%rax)
    movaps  .LCPI2_2(%rip), %xmm0
    movups  %xmm0, 56(%rax)
    movq    %rcx, 72(%rax)
    movaps  .LCPI2_3(%rip), %xmm0
    movups  %xmm0, 80(%rax)
    movq    %rcx, 96(%rax)
    movaps  .LCPI2_4(%rip), %xmm0
    movups  %xmm0, 104(%rax)
    movq    %rcx, 120(%rax)
    movaps  .LCPI2_5(%rip), %xmm0
    movups  %xmm0, 128(%rax)
    movq    %rcx, 144(%rax)
    movaps  .LCPI2_6(%rip), %xmm0
    movups  %xmm0, 152(%rax)
    movq    %rcx, 168(%rax)
    movq    %rax, %rdi
    callq   swift_setDeallocating@PLT
    leaq    (demangling cache variable for type metadata for (Swift.Int, Swift.String))(%rip), %rdi
    callq   __swift_instantiateConcreteTypeFromMangledName
    movl    $6, %esi
    movq    %rbx, %rdi
    movq    %rax, %rdx
    callq   swift_arrayDestroy@PLT
    movl    $21, %eax
    addq    $176, %rsp
    popq    %rbx
    retq

So I think that the optimiser doesn't actually quite understand that taking out whole arrays is fine because creating and destroying an array doesn't do any side effects that anybody could legally observe in Swift.

Repro:

swiftc -emit-assembly -O test.swift  | swift demangle | grep -A60 ^test.doIt
@weissi
Copy link
Member Author

weissi commented Jan 25, 2021

@swift-ci create

@eeckstein
Copy link
Member

Fixed in main with #35737

@weissi
Copy link
Member Author

weissi commented Feb 8, 2021

Very cool, thank you @eeckstein! Do you think that PR would be appropriate for 5.4 too at this time or is it "too risky"?

@eeckstein
Copy link
Member

No, unfortunately for 5.4 it's too late.

@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.
Projects
None yet
Development

No branches or pull requests

2 participants