Skip to content

[Exclusivity] Make the performance inliner aware of exclusivity checks #19894

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

Merged
merged 1 commit into from
Oct 16, 2018

Conversation

shajrawi
Copy link

radar rdar://problem/45283321

@shajrawi shajrawi requested a review from eeckstein October 15, 2018 22:17
@shajrawi
Copy link
Author

@eeckstein Can you please review?

@shajrawi shajrawi force-pushed the inline_exclusivity branch 5 times, most recently from 5ad7b32 to a626f4a Compare October 16, 2018 00:04
@shajrawi
Copy link
Author

@swift-ci Please test

@shajrawi
Copy link
Author

@swift-ci Please test Linux

@@ -407,6 +446,12 @@ bool SILPerformanceInliner::isProfitableToInline(
}
}

Weight EntryW = SPA->getWeight(CalleeEntry, CallerWeight);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you taking the weight of the entry block and not the weight of the block which contains the access(es)?

Copy link
Author

Choose a reason for hiding this comment

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

When we visit the block that contains the accesses we might not have visited the entire function / further block might change our decision. by the time we traverse all blocks we 'lost' which block(s) contain accesses. but I agree this is less than ideal - will think about changing it

@shajrawi shajrawi force-pushed the inline_exclusivity branch 3 times, most recently from 16ab56d to 1cdd6ba Compare October 16, 2018 19:57
@shajrawi
Copy link
Author

@eeckstein I changed the benefit calculation per our off-line discussion

@swiftlang swiftlang deleted a comment from swift-ci Oct 16, 2018
@shajrawi
Copy link
Author

@swift-ci Please smoke test and merge

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

@swift-ci swift-ci merged commit b0389af into swiftlang:master Oct 16, 2018
@shajrawi shajrawi deleted the inline_exclusivity branch April 12, 2019 22:42
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.

3 participants