swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.54k stars 10.35k forks source link

Struct being copied if used in the condition of an if statement multiple times #69994

Open nocchijiang opened 11 months ago

nocchijiang commented 11 months ago

Description A struct is copied if it is used in the condition expression of an if statement multiple times.

Steps to reproduce

import Foundation

protocol Foo {
    @inline(never)
    func foo(_ i: Int) -> Bool
}

struct Huge: Foo {
    var s1: String = "1"
    var i1: Int = 1
    var d1: Float = 1.0
    var a1: [String] = ["1"]
    var s2: String = "2"
    var i2: Int = 2
    var d2: Float = 2.0
    var a2: [String] = ["2"]
    var s3: String = "3"
    var i3: Int = 3
    var d3: Float = 3.0
    var a3: [String] = ["3"]
    var s4: String = "4"
    var i4: Int = 4
    var d4: Float = 4.0
    var a4: [String] = ["4"]
    var s5: String = "5"
    var i5: Int = 5
    var d5: Float = 5.0
    var a5: [String] = ["5"]
    var s6: String = "6"
    var i6: Int = 6
    var d6: Float = 6.0
    var a6: [String] = ["6"]
    var s7: String = "7"
    var i7: Int = 7
    var d7: Float = 7.0
    var a7: [String] = ["7"]
    var s8: String = "8"
    var i8: Int = 8
    var d8: Float = 8.0
    var a8: [String] = ["8"]
    var s9: String = "9"
    var i9: Int = 9
    var d9: Float = 9.0
    var a9: [String] = ["9"]
    var s10: String = "10"
    var i10: Int = Int.random(in: 0...100)
    var d10: Float = 10.0
    var a10: [String] = ["10"]

    @inline(never)
    func foo(_ i: Int) -> Bool {
        return (i + i10) % 2 == 0
    }
}

struct S {
    let foo: Foo

    @inline(never)
    func bar() -> Void {
        let foo = self.foo
        if foo.foo(Int.random(in: 0..<100)) || foo.foo(Int.random(in: 1..<10)) || foo.foo(Int.random(in: 2..<1000)) {
            print("hello")
        }
    }

}

let s = S(foo: Huge())
s.bar()

To compile the demo:

swiftc -O -sdk <path_to_mac_os_sdk_from_xcode> demo.swift

In the generated code, the "outlined init with copy of Foo" is called in S.bar() multiple times.

Expected behavior The foo struct should not be copied except for the let binding in S.bar().

Environment I have tested and confirmed this behavior exists on both Xcode 15.0.1 and the trunk development snapshot. Initially this issue was found in an iOS app, I made a minimum-reproducible demo for the issue report.

Azoy commented 11 months ago

In this case foo is not a struct, it is an existential whose underlying type we have no idea about. If S were generic over Foo it could specialize the function and only need to copy it once (via a retain).

nocchijiang commented 11 months ago

In this case foo is not a struct, it is an existential whose underlying type we have no idea about.

IIUC, you mean this is part of the price we have to pay by using existential? The side effect of using an existential in a slightly complex binary expression is to implicitly copy it? This doesn't sound right to me.

skyien commented 11 months ago

If I understand correctly, we have copies here

func bar() -> Void {
        if self.foo.foo(Int.random(in: 0..<100)) || self.foo.foo(Int.random(in: 1..<10)) || self.foo.foo(Int.random(in: 2..<1000)) {
            print("hello")
        }
}

but here - not?

func bar() -> Void {
        let c1 = self.foo.foo(Int.random(in: 0..<100))
        let c2 = self.foo.foo(Int.random(in: 1..<10))
        let c3 = self.foo.foo(Int.random(in: 2..<1000))
        if c1 || c2 || c3 {
            print("hello")
        }
}
nocchijiang commented 11 months ago

@skyien Yes, in the second code snippet, the calls to foo are moved out of the binary expression. My understanding of the issue is that:

  1. The frontend generates implicit closure for nested binary expressions. I think this is fine, but it is still part of the cause.
  2. When lowering the closure, the compiler somehow decided that a noespace capture needs to be copied.
tbkka commented 11 months ago

CC: @eeckstein @nate-chandler

eeckstein commented 11 months ago

cc @atrick The copy_addr is already generated by SILGen. But okay, SILGen is not very smart. But it should be easily possible for CopyForwarding to eliminate the copy because the destination is either used by an @in_guaranteed partial-apply argument or by an open_existential_addr immutable_access

nocchijiang commented 10 months ago

I found another bad case in production that self gets copied but I cannot reproduce it in a complete demo (I don't know how to make an address-only struct in the demo - I have confirmed that this is one of the necessary conditions). In the foo getter, self is being copied in foo multiple times if AHugeStruct is address-only.

struct AHugeStruct {
    let anEnum: AnEnum

    var foo: Bool {
        return anEnum != .caseA &&
               anEnum != .caseB &&
               anEnum != .caseC &&
               anEnum != .caseD
    }
}
atrick commented 10 months ago

@nocchijiang to make a struct address only, you can

  1. define the struct in another module
  2. add a generic member to the struct struct AHugeStruct<T> { var t: T }