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

[FEATURE] Support Clang compilation #222

Open
mikeoliphant opened this issue Apr 26, 2023 · 6 comments
Open

[FEATURE] Support Clang compilation #222

mikeoliphant opened this issue Apr 26, 2023 · 6 comments
Labels
enhancement New feature or request good first issue Good for newcomers priority:low Low priority issues

Comments

@mikeoliphant
Copy link
Contributor

My testing indicates that compiling with Clang gives a noticeable (on the order of 10%) speed improvement - likely because it optimizes the Eigen matrix code a bit better than the VS compiler.

It probably makes sense not to switch the VS project to Clang, since it will make it harder for people to compile out-of-the-box (most people won't have Clang installed). I think it can be specified in the release build process, though.

The only issues I encountered with building the plugin with Clang were a couple of minor iPlug2 things that seem to have already been fixed - but in a later version of iPlug2 than the fork the plugin is currently using.

@mikeoliphant mikeoliphant added enhancement New feature or request priority:low Low priority issues unread This issue is new and hasn't been seen by the maintainers yet urgent:no labels Apr 26, 2023
@olilarkin
Copy link
Contributor

I am not sure why NAM is using a non-fork fork of iPlug2 :)

@olilarkin
Copy link
Contributor

I think this can be closed since the repo now tracks the iPlug2 repo, where iPlug2/iPlug2@dbdf118 is merged

@mikeoliphant
Copy link
Contributor Author

I think this can be closed since the repo now tracks the iPlug2 repo, where iPlug2/iPlug2@dbdf118 is merged

That should hopefully enable clang support, but we still don't actually use it. If it has a significant performance impact (as it seems to), we should consider using it for the build.

@sdatkinson sdatkinson removed the unread This issue is new and hasn't been seen by the maintainers yet label May 6, 2023
@sdatkinson sdatkinson added the good first issue Good for newcomers label May 23, 2023
@olilarkin
Copy link
Contributor

i think this can be closed now?

@mikeoliphant
Copy link
Contributor Author

@sdatkinson Can you re-open this?

I think it is worth looking into having an official clang build. Recent experiments with clang on the LV2 plugin are showing around a 30% performance improvement for standard WaveNet models:

mikeoliphant/neural-amp-modeler-lv2#51

@sdatkinson sdatkinson reopened this Oct 21, 2023
@mikeoliphant
Copy link
Contributor Author

PR here that gets a start on this: #383

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers priority:low Low priority issues
Projects
None yet
Development

No branches or pull requests

3 participants