FFTW / fftw3

DO NOT CHECK OUT THESE FILES FROM GITHUB UNLESS YOU KNOW WHAT YOU ARE DOING. (See below.)
GNU General Public License v2.0
2.66k stars 651 forks source link

Preliminary PR to merge ARM SVE branch #315

Open rdolbeau opened 1 year ago

rdolbeau commented 1 year ago

This is primarily for discussion ad probably need some cleaning up.

The current scheme using SVE (and RISC-V V) doesn't leverage the scalability; instead, it produces sets of codelets for all possible power-of-2 sizes, using masking. Only sets of a size <= to the hardware implementation width are enabled. See here.

It also auto-generate the simd-support/vtw.h file that defines the various VTW macro. This is useful as while SVE is limited to 2048 bits wide vector, RISC-V V is more or less unbounded and there's a least a 16384 bits wide implementation being developed.

So using SVE (or V) creates a fairly large, though versatile, library.

I has been tested in QEmu; using the Arm instruction Emulator; and on real hardware (Fujitsu A64FX, AWS Graviton 3).

rdolbeau commented 1 year ago

I've created release package for testing this w/o the 'maintainer mode' requirements. See there: https://github.com/rdolbeau/fftw3/releases/tag/sve-test-release-001

rdolbeau commented 7 months ago

@stevengj @matteo-frigo Can you comment on this? In particular the "used fixed-width implementation and enable all those of equal-or-narrower to the hardware width". It's a bit hackish but it's the best we have at this time. RISC-V is using the same principle for now.

ggouaillardet commented 4 months ago

I would like to encourage the FFTW folks to consider this PR so it can be included (after some cleanup) into the next FFTW release.

At this stage, my main interest is only about ARM SVE. SVE enabled processors are currently featured by several vendors:

Though SVE vector length can scale from 128 to 2048 bits (via 128 bits increments), only 3 sizes are available today (128, 256 and 512. Fujitsu A64fx does not support 384 bits). So even if vector length agnostic FFTW might be achievable, I do not see this as mandatory because of the currently limited available vector lengths and the fact VLA code is (generally) slightly slower than vector length specific code. If needed, I can provide an option to set a maximum supported vector length in order to reduce the size of the FFTW library.

Bottom line, I approve the approach of the PR and hope it can make its way upstream.

rdolbeau commented 4 months ago

Though SVE vector length can scale from 128 to 2048 bits (via 128 bits increments), only 3 sizes are available today (128, 256 and 512. Fujitsu A64fx does not support 384 bits)

Also, in its current incarnation SVE doesn't allow for non-power-of-2 multiple of 128 bits anymore, only power-of-2 multiple of 128 bit are. This is visible for instance in DDI0487J for the register ZCR_EL1: "The Non-streaming SVE vector length can be any power of two from 128 bits to 2048 bits inclusive."

So this implementation should cover all cases, and I agree with you that 1024 and 2048 bits are currently only for emulators like QEmu and could (should?) be optional. 512 and 256 could also be optional for site-specific deployment.

rdolbeau commented 4 months ago

Done some minor clean-ups, minor improvement, and rebase to current HEAD.

@ggouaillardet if you have the time to confirm this is still OK for you

rdolbeau commented 4 months ago

I've created a new package with pre-generated files, so maintainer mode is not required to test this on Arm+SVE, see https://github.com/rdolbeau/fftw3/releases/tag/sve-test-release-002

ggouaillardet commented 2 months ago

Thanks @rdolbeau and my apologies for the late reply.

I noted configure --enable-sve works but make fails out of the box with Arm compilers, so I added a test in order to abort at configure instead of failing at make. Feel free to cherry-pick ggouaillardet/fftw3@c32583cbb7ff30dd9e53374b78356f08d4c95466

rdolbeau commented 2 months ago

I noted configure --enable-sve works but make fails out of the box with Arm compilers

Probably with all compilers that do not enable SVE by default (all of them, probably!), currently configure doesn't enable it explicitly so it has to be done in {C/CXX/F}FLAGS.

so I added a test in order to abort at configure instead of failing at make. Feel free to cherry-pick ggouaillardet/fftw3@c32583c

Good idea. I'll try that and add it to the PR ASAP.

juntangc commented 2 months ago

Though SVE vector length can scale from 128 to 2048 bits (via 128 bits increments), only 3 sizes are available today (128, 256 and 512. Fujitsu A64fx does not support 384 bits)

Also, in its current incarnation SVE doesn't allow for non-power-of-2 multiple of 128 bits anymore, only power-of-2 multiple of 128 bit are. This is visible for instance in DDI0487J for the register ZCR_EL1: "The Non-streaming SVE vector length can be any power of two from 128 bits to 2048 bits inclusive."

So this implementation should cover all cases, and I agree with you that 1024 and 2048 bits are currently only for emulators like QEmu and could (should?) be optional. 512 and 256 could also be optional for site-specific deployment.

I can see performance benefits by extending to 256 bits and above. Do you happen to have performance data for the sve version comparing to the SIMD NEON version?

ggouaillardet commented 2 months ago

Here is some performance number I collected a while ago for fftw_plan_dft_1d(signal_length, original_signal, fft_applied_signal, FFTW_FORWARD, FFTW_ESTIMATE) It shows the performance improvement between NEON and SVE on A64fx processor (512 bits SVE vectors)

fftw

Feel free to suggest other/better benchmarks if appropriate.

antoine-morvan commented 2 months ago

Hello,

I ran few experiments using the included benchmark tests/bench, built with double support. My commands were binding the execution to the 4th core on each of the machines (arbitrary choice) :

numactl -C 3 ./tests/bench -v2 -r 20 -s $PB_DEF

Where PB_DEF within the few benchmark settings ocf2048, icf32x32, ocf128x128x128, icf256x256x256 .

image

Baseline stands for the current release 3.3.10 built with --enable-neon, and the SVE gain is this PR built with --enable-neon --enable-sve (on Neoverse N1 only --enable-neon was used). The charts are comparing the "mflops" value output by the benchmark utility.

We can observe significant gain on all SVE machines (A64FX, Neoverse V1 and V2), as well as no degradation on NEON only machine (Neoverse N1).

Best regards.

jlinford commented 1 month ago

These results are very exciting. FFTW performance is extremely important to users of NVIDIA Grace (Neoverse V2), which is now the dominant architecture on the Green500. A great many people would be glad to see this commit in the next FFTW release.

rdolbeau commented 6 days ago

I've created a new package with pre-generated files, so maintainer mode is not required to test this on Arm+SVE, see https://github.com/rdolbeau/fftw3/releases/tag/sve-test-release-003

rdolbeau commented 6 days ago

@stevengj @matteo-frigo any comment on this? Is this OK to think about merging ?