Skip to content

[AST] Perf: Make generic decl types share field offsets and fast casting logic #13663

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

Conversation

davezarzycki
Copy link
Contributor

No description provided.

@davezarzycki
Copy link
Contributor Author

Hi @DougGregor – Are you okay with these added abstract classes? They help improve code gen by ensuring that "TheDecl" is stored at the same offset in memory and the change makes classof() efficient when checking for "any generic" or "any nominal or bound generic nominal". Also, I could surely improve the comments describing these abstract classes. Is there anything you would want said or not said? Thanks!

@swift-ci please test

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

LGTM. I'm fine with the extra step in the hierarchy if it improves code-gen

union {
GenericTypeDecl *GenDecl;
NominalTypeDecl *NomDecl;
};
Copy link
Member

Choose a reason for hiding this comment

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

That's a clever bit of aliasing here... but it'll work fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Ya, the union is gross. I tried using templates, but that fought with the CanType macros. I wasn't sure what was more important. It should be polished at some point.

@davezarzycki davezarzycki merged commit 07f5029 into swiftlang:master Jan 2, 2018
@davezarzycki davezarzycki deleted the nfc_refactor_typenode_hierarchy branch January 2, 2018 21:02
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.

2 participants