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

(WIP) Add basic support for Roslyn Analyzers #1120

Closed
wants to merge 11 commits into from

Conversation

Yey007
Copy link
Contributor

@Yey007 Yey007 commented Nov 11, 2023

Hello! This is a proof-of-concept for shipping Roslyn Analyzers with ILGPU. The intent is to build a framework to easily add analyzers in the future, and provide a basic one that covers one of the common beginner mistakes with ILGPU: using reference types in the kernel.

TODO:

  • A number of things to improve with the analysis itself, outlined in the code
  • Add tests
  • Add more samples
  • Remove remaining extraneous files, like generated .Designer.cs files
  • Add analyzers to CI/CD pipeline
  • Add analyses to AnalyzerReleases.Shipped.md
  • Improve README.md

@Yey007 Yey007 marked this pull request as draft November 11, 2023 05:58
@Yey007
Copy link
Contributor Author

Yey007 commented Nov 18, 2023

A couple of questions for discussion that came up while implementing the reference type analyzer specifically:

  1. Should errors trace back to the point reference types are instantiated? Or should they pop up anywhere they're used?
  2. Similarly, should invoked methods that use reference types be highlighted at the invocation as well as at the usage of the reference type? Currently this is not the behavior.
  3. How much effort do we want to put into discovering methods that are kernels? Dataflow analysis can probably be used to achieve somewhat good coverage, depending on the current implementation of the API in Roslyn. I haven't looked much into how it works.
  4. Does having KernelAnalyzer as a base class seem useful? I don't know how many of our analyzers will really require the behavior of going through every line of code potentially accessible from a kernel.

@Yey007
Copy link
Contributor Author

Yey007 commented Dec 28, 2023

Closing in favor of rewriting changes on top of #982.

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.

1 participant