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

missed optimization opportunity for elimination of retain/release and tail recursion #58549

Open
RonAvitzur opened this issue Apr 29, 2022 · 1 comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior.

Comments

@RonAvitzur
Copy link

RonAvitzur commented Apr 29, 2022

Describe the bug
Unnecessary retain/release seem to be inhibiting tail recursion optimization in

var leftMost: Expr { args.count == 0 ? self : args.first!.leftMost }

final class Expr {
    unowned(unsafe) var parent: Expr? = nil
    var args: ContiguousArray<Expr> 
}

To Reproduce
Compiling the above generated a recursive function with retain/release around each step.

Expected behavior
I expected the retain/release to be optimized away and the recursive call to be reduced to an iteration.

Environment (please complete the following information):

  • OS: 12.3.1 (21E258)
  • Xcode 13.3.1 (13E500a)

Additional context

(I think I checked this in a previous version of Xcode about a year ago, and at that time the compiler did do a tail recursion optimization, so this behavior may be a regression. However, my memory may be faulty confusing this with something else, and I can't remember which previous compiler version to check against.)

Discussion and disassembly are at https://forums.swift.org/t/arc-tail-recursion/57054

Writing the function without the tail recursion still leaves the retain/release around each step.

    var leftMost: Expr {
        var expr = self
        while expr.args.count > 0 { expr = expr.args.first! }
        return expr
    }

Jordan Rose writes at https://forums.swift.org/t/arc-tail-recursion/57054/7

Swift will not let you do anything with a value without it being retained, or without a promise that someone else is retaining it in the mean time. There aren't many generally supported ways to spell the latter if it can't be inferred from the structure of the code.

In this case, the code in question is enough to guarantee that, since the compiler can see that indeed no mutations can happen at all here. So I'd say this is a missed optimization opportunity and leave it at that.

@RonAvitzur RonAvitzur added the bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. label Apr 29, 2022
@eeckstein
Copy link
Contributor

The first problem is that the compiler in Xcode 13.3 does not inline the ContiguousArray.subscript getter co-routine.
That seems to be fixed on main.

But the retain-release pair around the recursive call is still there and we need to improve inter-procedural side-effect analysis to be able to handle this. Not sure why and if an older compiler could optimize this.

rdar://93912198

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