Add #[simd] macro

[Shnatsel]

My attempt at #338

I am not well-versed in proc macros. While the design of what the macro should expand into is mine, the implementation of the macro and thinking through various edge cases and handling them was largely delegated to Codex (GPT-5.6 Sol Ultra).

After the initial AI implementation I moved all soundness-critical parts into a declarative macro, which does all the interesting stuff, and which I find a lot easier to understand. The proc macro is just there as a non-soundness-critical convenience that forwards arguments to the declarative macro.

Before implementing the macro I investigated vectorize() and found that it already gets inlined most of the time anyway. It doesn't work well as an inlining barrier, since the function that isn't annotated #[inline] is trival enough for the optimizer to always inline it anyway, even with a large closure body. So I reverted a separate vectorize_inline() which is recorded in commit history, and inlining can instead be controlled with annotation on the function calling vectorize(); the docs on it already show that usage in the example, the doc comment on vectorize() can probably be improved.

The proc macro crate is separate from fearless_simd so it doesn't fall under the v1.0 guarantees. I expect to publish it in lockstep with v1.0, although we might ship pre-releases just to make sure it all works. The README points to git but should work with most versions no problem, just wouldn't have as much inlining annotations in vectorize().

[LaurenzV]

I think let's wait until linebender/vello#1853 has been merged and then we can try the ergonomics of using it there?

[Dr-Emann]

Woudln't mind the #[inline] on the inside of vectorize (and/or eliminating the inner call when at baseline), I'm seeing some useful performance impact on aarch64 by doing it kinda manually with:

#[cfg(all(target_arch = "aarch64", target_feature = "neon"))]
if simd.level().as_neon().is_some() {
    return inner_code(a, b, c);
}
simd.vectorize(
    #[inline(always)]
    move || inner_code(a, b, c);
}

I believe it's somewhat exaggerated by taking enough arguments, in:

        #[target_feature(enable = "neon")]
        fn vectorize_neon<F: FnOnce() -> R, R>(f: F) -> R {
            f()
        }
        unsafe { vectorize_neon(f) }

vectorize_neon gets a single closure struct containing a, b, and c, which spills to the stack, rather than being passed as registers which passing as arguments can do.

Wonder if there's some macro magic we could do in the #[simd] impl that could use real arguments, rather than delegating to .vectorize(closure)

[Dr-Emann]

And/or, maybe just special casing a few arguments, and doing an annoying simd.vectorize_1(a, #[inline(always)] |a| { ... }), simd.vectorize_2(a, b, #[inline(always)] |a, b| { ... }), ...

[Shnatsel]

Ugh, turns out closure-captured arguments get put into a struct that then gets spilled to the stack.

I've rewritten the macro to avoid that with Astra's assistance.

The proc macro now also needs to refer to fearless_simd crate explicitly, and to avoid making it unsound by spoofing fearless_simd crate I had to move all the unsafe into a declarative macro inside fearless_simd itself.

I think it's a reasonable split going forward, especially since all the level matching is now happening inside fearless_simd where it can all be updated in lockstep if/when we add new levels. Alternatively we can make renaming fearless_simd harder by using ::fearless_simd in the proc macro output and this will require renaming it in Cargo.toml; technically a soundness hole if you do that, but not a practical one.

[Shnatsel]

tests fail because trybuild is too strict and the changes to the compiler's diagnostic output between versions trip it up. The current snapshots expect 1.97 output while CI runs on 1.89.

There is no fuzzy matching functionality in trybuild so we might have to use something else for testing the macro.

[RunDevelopment]

Since you wanted the PR reviewed, I spent some time reading through it. Far from a proc macro expert, but I have some experience with it.

[RunDevelopment]

That's not true yet. #379

[RunDevelopment]

requires

Not really part of the review, but I want to give some feedback.

I take issue with "requires" here and the general framing of #[simd]/Simd::vectorize/#[inline(always)] being necessary for SIMD. Technically, you don't need to use any of them for SIMD. Example:

#[inline(never)] // no inlining to make a point
fn add<S: Simd>(a: f32x4<S>, b: f32x4<S>) -> a: f32x4<S> {
    a + b
}

This function will work (=correct result), and the addition will indeed use whatever SIMD instruction S allows to perform the addition. The only issue is that it is going to be slow. Without any target features, the internal function that actually executes the SIMD instruction can't be inlined. So you pay for a whole function call + missing optimization opportunities to run (probably) a single instruction. Ouch.

This really tripped me when I first used fearless_simd. I read your docs, but it wasn't 100% clear to me why #[inline(always)] and the likes were strictly necessary. My code worked, tests passed. Took me some time to figure out why the fearless AVX2 version of the encoder was slower than the previous version. I only figured it out after looking at the assembly of my program.

So, please, don't just document how to use fearless correctly; also show what happens if you don't. Show some assembly and benchmarks to emphasize how bad it is. As a user, I can say that it's easy with this library to "hold it wrong." #[simd] is going to help a lot in that regard (because it's easy to use), but at least the section on manual inlining should drive home what the consequences for improper usage are.

[Shnatsel]

Great point, thank you!

It can be difficult to know what exactly needs explaining when you already have the whole system in your head, so this kind of feedback is very valuable!

[Shnatsel]

I've added a note on what happens if you omit #[simd], how's this?

[RunDevelopment]

Short and simple. It's good.

I would just go into my detail why it is slow in the section about manual inlining. But assuming that only experts will use manual inlining, maybe it's not necessary?

[RunDevelopment]

Found 2 minor issues after using the macro.

[LaurenzV]

Thanks a lot! I just have a single question/concern regarding the usage of the simd macro. I have not tried to validate all of the edge cases of the macro itself, but clearly you've already invested a lot of time into finding those!

I'm just going to rerun the vello benchmarks on AVX2 and if it all looks good happy to approve.

Also, three comments from codex, they seem to be accurate from that I can tell:

[LaurenzV]

So, I checked again on your vello branch, and I saw that for gradients, you removed the simd macro and instead replaced it with inline always. And indeed, when I tried using the SIMD macro instead of inline always, I got worse performance.

The documentation makes it sound like if you are fine using #[simd] you can just always use this and don't have to worry about anything anymore, but clearly that doesn't seem to apply in some cases. But I'm wondering whether there is any rule to that and how we can convey that to users somehow? Or is it possible for the simd macro to take an additional inline_always attribute or something, so that the user still has some control over this behavior if we can't make this issue go away "magically"?

[LaurenzV]

I tried putting inline(always) on top of the methods and it recouped the performance loss for radial gradients, but not sweep gradients. But maybe that's enough. Should we document that it might be worth combining with inline always (or that it's supported in the first place)?

Do we need to document that, in case it doesn't work, users can still try not using the simd annotation and just marking the method as inline always>

[Shnatsel]

This is the usual messing around with inlining that I don't think the presence of #[simd] really changes. So I'm not sure how to document it succinctly.

It's not like the #[simd] macro didn't do its job in that case. It does, and the gradient functions still emit SIMD instructions. The compiler simply decided to keep it a separate function based on its heuristics, and that happened to reduce performance in this specific case.

[LaurenzV]

Okay, fair enough!

[Shnatsel]

On Codex critique:

1. This is valid, but would require a great deal of additional complexity to support. I don't really want to go down that road.
2. Fixed in 0488084 and made the code simpler, nice!
3. Good catch, fixed!

[Shnatsel]

I'm excited to have this finally merged, thank you both @RunDevelopment and @LaurenzV for the review!