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

Support static abstract interface members #930

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

lostmsu
Copy link
Contributor

@lostmsu lostmsu commented Feb 13, 2023

This replaces specific generic math support introduced in 7bf5578 ( #898 ) with generic support for static abstract interface members.

Support is implemented using the constrained flag on the call instruction the same way it used to be done on virtual method calls. The call instruction with constrained flag set contains concrete type, that implements given interface, which makes it possible to find concrete method implementation to be called.

fixes #929

Copy link
Collaborator

@MoFtZ MoFtZ left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @lostmsu.

This is a much better implementation than the previous usage of reflection.

Src/ILGPU/Frontend/CodeGenerator/Calls.cs Outdated Show resolved Hide resolved
Src/ILGPU/Frontend/CodeGenerator/Calls.cs Show resolved Hide resolved
@MoFtZ MoFtZ added this to the v1.4 milestone Feb 14, 2023
@lostmsu lostmsu force-pushed the StaticAbstractInterfaceMembers branch from 5203332 to c8ef126 Compare February 14, 2023 03:56
Copy link
Collaborator

@MoFtZ MoFtZ left a comment

Choose a reason for hiding this comment

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

hi @lostmsu.
Looks like the CI pipeline is failing because the copyright headers have not been updated.

The following command should update the necessary files:
dotnet run --configuration=Release -p:TreatWarningsAsErrors=true --project Tools/CopyrightUpdateTool,

@lostmsu lostmsu force-pushed the StaticAbstractInterfaceMembers branch 2 times, most recently from b2ab3a3 to 7803cf8 Compare February 14, 2023 18:24
@lostmsu
Copy link
Contributor Author

lostmsu commented Feb 14, 2023

@MoFtZ hm, I don't seem to be able to reproduce unit test failures locally. Besides, before the copyright year change they were passing???

@MoFtZ
Copy link
Collaborator

MoFtZ commented Feb 14, 2023

@lostmsu Looks like there is a new version of the .NET 7 SDK. CI pipeline used to work on 7.0.102, and is now failing on 7.0.200.

UPDATED: I've raised #932 to track this issue.

This replaces specific GenericMath support introduced in 7bf5578 ( m4rs-mt#898 )  with generic support for static abstract interface members.

Support is implemented using the constrained flag on the call instruction the same way it used to be done on virtual method calls. The call instruction with constrained flag set contains concrete type, that implements given interface, which makes it possible to find concrete method implementation to be called.

fixes m4rs-mt#929
@lostmsu lostmsu force-pushed the StaticAbstractInterfaceMembers branch from 7803cf8 to 1daae30 Compare February 15, 2023 18:43
@lostmsu
Copy link
Contributor Author

lostmsu commented Feb 15, 2023

Updated with the latest master that should have fixed the test failures.

@m4rs-mt
Copy link
Owner

m4rs-mt commented Feb 16, 2023

Thanks a lot for you work on this issue 👍 !

@m4rs-mt m4rs-mt merged commit 851670d into m4rs-mt:master Feb 16, 2023
@lostmsu lostmsu deleted the StaticAbstractInterfaceMembers branch May 1, 2023 20:50
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.

Abstract static interface methods support is incomplete
3 participants