ValleyAudio/ValleyRackFree

No arm64 as SSE ...

Closed this issue · 18 comments

arch specifics.

Builds fine on arm64 (mac) if you comment out the offending header file from all the hpp files that explicitly include it

I'm currently porting the plugin to use the Rack SIMD functions so that I will no longer need to uae raw Intel Instrinsics. The build system should then generate a universal binary.

@ValleyAudio theres no problem using sse intrinsics on the arm build you just need to replace your sse includes with appropriate simde or intel ones.

If you replace your intrinsic includes with rack.h it just works but if you don’t want to couple to rack at that point in your code you can also do something like this:

https://github.com/surge-synthesizer/surge/blob/2976336e108ba0f86718f03942715287122c7bd3/src/common/globals.h#L49

hope that helps!

@baconpaul You know what, that's not a bad shout. I'll just include simde instead of porting it to use rack::simd

That's what I did with surge on raspberry pi 3 years ago and it's worked great every day since :)

Screen Shot 2023-08-20 at 6 19 05 PM

Shoot I should have just done it. Took me five minutes. Here's valley on my arm mac.

works.patch

Here's the patch. You may want to make a neutral include file others use to avoid the splattage

I have the docker env up and that built all platforms too (both mac, lin, and win)

Really just tying to stop arm users from using Surge Reverb 2 :)

pgatt commented

@ValleyAudio Hi Dale, hope you're well, I'm wondering if there is any chance you could finalise your plugin for ARM64? Lots of people have contacted VCV Support requesting them.

@pgatt Hiya, I've been terribly busy with co-chairing the recent Audio Engineering Society conference and then being involved in my choir's recent Proms performance, so today is the first day that I could find to sit down and make the edits.

I'm currently fixing issues made by brew in my dev environment for Rack, but once those are sorted I can then make the edits you suggested.

Apologies for the delay.

pgatt commented

@ValleyAudio Thanks Dale, we look forward to seeing them!

If it helps I can clean up my patch today and send it as a pr. Lemme know!

@baconpaul I completely missed that you had done that, Git Hub on mobile compress all the comments. If that job only took 5 mins and is similar to what @pgatt suggested, then yeah please make PR!

OK PR is in, above.

@baconpaul @pgatt Okay, merged the PR, but also stuck a commit from my private repo on top of this that includes a lot of refactoring and general housekeeping. Once I've bumped the version number and I'm satisfied that it meets the requirements, I will submit.

Awesome

Hi @pgatt and @baconpaul. If either of you have access to Windows and / or Linux, could you please test my recent commit?

First, your commit added another intrinsic. I just popped in a PR which I think you should merge.

When I build in the toolchain I get a linux build but get a windows error which I haven't diagnosed yet. (Have you ever considered setting up GitHub pipelines on this repo to check that?)

The windows thing is a problem with mac arm and docker toolchain; you can ignore it. I'll chat with christophe about it.

the code with my PR applied builds fine everywhere!

@baconpaul Cool, thanks for checking and making the PR. I'll review it tomorrow morning. I have thought about adding CI to this repo to make things easier, so thanks for the suggestion.