Skip to content

[ownership] Enable GlobalOpt on ossa and add ossa tests #32621

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 4 commits into from
Jul 7, 2020

Conversation

meg-gupta
Copy link
Contributor

@meg-gupta meg-gupta commented Jun 30, 2020

GlobalOpt works mostly on trivial values (there are special cases for ObjectInst and ValueToBridgeObjectInst).
optimizeGlobalAccess is explicitly turned off for non-trivial values. optimizeInitializer calls SILGlobalVariable::isValidStaticInitializerInst which limits it to mostly trivial values except for special cases for ObjectInst and ValueToBridgeObjectInst.

This changes adds GlobalOpt tests for ossa and enables GlobalOpt on ossa

@meg-gupta meg-gupta requested a review from gottesmm June 30, 2020 16:40
@meg-gupta meg-gupta requested a review from atrick July 6, 2020 17:33
@meg-gupta
Copy link
Contributor Author

@swift-ci test

@swift-ci
Copy link
Contributor

swift-ci commented Jul 6, 2020

Build failed
Swift Test Linux Platform
Git Sha - 73bfd8f28be05934a362af4c0c78b0bf1bcdb68b

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

I'm not sure what's being tested. Can you please add some comments to globalopt_trivial_nontrivial.sil?


@_hasStorage @_hasInitialValue let trivialglobal: TStruct { get }

@_hasMissingDesignatedInitializers public class TClass {
Copy link
Contributor

Choose a reason for hiding this comment

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

You may need to move extraneous attributes like @_hasMissingDesignatedInitializers and @objc so the test works on non-mac platforms

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@meg-gupta
Copy link
Contributor Author

@swift-ci smoke test

@meg-gupta
Copy link
Contributor Author

@atrick Added comments to globalopt_trivial_nontrivial.sil. I added this test just to show no-op of global opt on non-trivial values.

Copy link
Contributor

@atrick atrick left a comment

Choose a reason for hiding this comment

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

Thanks!

@meg-gupta meg-gupta merged commit 9c5c733 into swiftlang:master Jul 7, 2020
eeckstein pushed a commit to eeckstein/swift that referenced this pull request Oct 29, 2020
)

GlobalOpt works mostly on trivial values (there are special cases for ObjectInst and ValueToBridgeObjectInst).
optimizeGlobalAccess is explicitly turned off for non-trivial values. optimizeInitializer calls SILGlobalVariable::isValidStaticInitializerInst which limits it to mostly trivial values except for special cases for ObjectInst and ValueToBridgeObjectInst.

This changes adds GlobalOpt tests for ossa and enables GlobalOpt on ossa
tbkka pushed a commit that referenced this pull request Oct 30, 2020
)

GlobalOpt works mostly on trivial values (there are special cases for ObjectInst and ValueToBridgeObjectInst).
optimizeGlobalAccess is explicitly turned off for non-trivial values. optimizeInitializer calls SILGlobalVariable::isValidStaticInitializerInst which limits it to mostly trivial values except for special cases for ObjectInst and ValueToBridgeObjectInst.

This changes adds GlobalOpt tests for ossa and enables GlobalOpt on ossa

Co-authored-by: Meghana Gupta <meghanavgupta@gmail.com>
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