swiftlang / swift

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

[performance] copy_addr is not optimized out for a C++ subscript invocation #61499

Open hyp opened 1 year ago

hyp commented 1 year ago

Swift has a redundant copy of the self parameter when a C++ subscript getter is invoked. This is causing signification slowdown for the std.vector subscript (as seen in https://github.com/apple/swift/pull/61456).

Steps To Reproduce test.h:

#include <stddef.h>

class MySubscriptable {
public:
  inline MySubscriptable(size_t sz) noexcept : values(new int[sz]) {
      for (size_t i = 0; i < sz; ++i) values[i] = 0;
  }
  inline MySubscriptable(const MySubscriptable &other) {
      values = other.values;
  }

  inline int operator[](size_t i) const noexcept { return values[i]; }
private:
  int *values;
};

inline int subscr(const MySubscriptable& v, size_t i) noexcept { return v[i]; }

module.modulemap:

module Testi {
  header "test.h"
  requires cplusplus
}

test.swift:

import Testi

@inline(never)
func blackHole(_ i: CInt) {
    print(i)
}

func test() {
    let v = MySubscriptable(100)

    let x = v[0]
    blackHole(x)

    let y = subscr(v, 0)
    blackHole(y)
}

test()

$ bin/swiftc -Xfrontend -enable-experimental-cxx-interop -I . -O test.swift -sdk $(xcrun --show-sdk-path) -o swift_test_bin -emit-sil -o sil.ir

=> sil.ir has copy_addr for the call to subscript, but not subscr:

%0 = alloc_stack [lexical] $MySubscriptable, let, name "v" // users: %21, %20, %17, %8, %4
...
 %7 = alloc_stack [lexical] $MySubscriptable     // users: %11, %10, %13, %8
  copy_addr %0 to [initialization] %7 : $*MySubscriptable // id: %8
  // function_ref _ZNK15MySubscriptableixEm
  %9 = function_ref @_ZNK15MySubscriptableixEm : $@convention(cxx_method) (Int, @in_guaranteed MySubscriptable) -> Int32 // user: %10
  %10 = apply %9(%6, %7) : $@convention(cxx_method) (Int, @in_guaranteed MySubscriptable) -> Int32 // users: %15, %12
  destroy_addr %7 : $*MySubscriptable             // id: %11
  debug_value %10 : $Int32, let, name "x"         // id: %12
  dealloc_stack %7 : $*MySubscriptable            // id: %13
...
  %16 = function_ref @_Z6subscrRK15MySubscriptablem : $@convention(c) (@in_guaranteed MySubscriptable, Int) -> Int32 // user: %17
  %17 = apply %16(%0, %6) : $@convention(c) (@in_guaranteed MySubscriptable, Int) -> Int32 // users: %19, %18

Expected behavior MySubscriptable is not copied on subscript.

hyp commented 1 year ago

TempRValueOpt pass can optimize a similar Swift copy but not a C++ copy

atrick commented 1 year ago

At first glance, I would expect this to be handled by

commit 13b9915c6fed5c1aefe1c93d89a8b69cad736205 Date: Tue Mar 10 12:23:02 2020 -0700

Use in_guaranteed for let captures (#29812)

@meg-gupta if you have time you may want to take a quick look at this case

hyp commented 1 year ago

TempRValueOpt says that the apply may write to memory, as this can return true:

-> 361            aa->mayWriteToMemory(inst, copySrc)) {
hyp commented 1 year ago

and that prevents the optimization

hyp commented 1 year ago

If we can infer @_effects(readonly) on the C++ subscript this would be fixed.

hyp commented 1 year ago

Potential fix - https://github.com/apple/swift/pull/61502

hyp commented 1 year ago

Fix - https://github.com/apple/swift/pull/61504