Skip to content

Conversation

@jkshtj
Copy link
Contributor

@jkshtj jkshtj commented Oct 6, 2023

For linear maps containing control-flow, closures (representing the pullbacks of intermediate values) may be passed as arguments, however, they may be hidden behind a branch-tracing enum (tracing execution flow of the original function).

Such linear maps did not use to get inlining benefits as the compiler could not see that the intermediate pullback closures were actually part of the input.

This change modifies the inliner logic to correctly award inlining benefits to linear maps containing control-flow, by checking if a "callee" in the linear map actually traces back to an input closure that was received as part of a branch-tracing enum input argument.

Fixes #68945

@jkshtj
Copy link
Contributor Author

jkshtj commented Oct 6, 2023

@BradLarson @asl could one of you please kick off swift-ci?

@BradLarson
Copy link
Contributor

@swift-ci Please test

@jkshtj
Copy link
Contributor Author

jkshtj commented Oct 6, 2023

@eeckstein do you think you could also take a look at this PR?

…maps w/ control-flow

For linear maps containing control-flow, closures (representing the pullbacks
of intermediate values) may be passed as arguments, however, they may be
hidden behind a branch-tracing enum (tracing execution flow of the original
function).

Such linear maps did not use to get inlining benefits as the compiler
could not see that the intermediate pullback closures were actually part
of the input.

This change modifies the inliner logic to correctly award inlining
benefits to linear maps containing control-flow, by checking if a
"callee" in the linear map actually traces back to an input closure that
was received as part of a branch-tracing enum input argument.

Fixes swiftlang#68945
Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@eeckstein
Copy link
Contributor

@swift-ci test

@jkshtj
Copy link
Contributor Author

jkshtj commented Oct 9, 2023

I think the windows build might need to be retried. It seems like the build timed out?

@eeckstein
Copy link
Contributor

@swift-ci test

1 similar comment
@eeckstein
Copy link
Contributor

@swift-ci test

@eeckstein
Copy link
Contributor

@swift-ci benchmark

@eeckstein eeckstein merged commit 0ada2e2 into swiftlang:main Oct 12, 2023
jkshtj added a commit to jkshtj/swift that referenced this pull request Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[AutoDiff] Check the inlining cost / benefit model for autodiff-generated functions (we need to ensure they receive benefit bonus)

3 participants