Skip to content

Make mask types opaque#218

Merged
Shnatsel merged 9 commits into
linebender:mainfrom
Shnatsel:opaque-mask-representation-minimal
May 23, 2026
Merged

Make mask types opaque#218
Shnatsel merged 9 commits into
linebender:mainfrom
Shnatsel:opaque-mask-representation-minimal

Conversation

@Shnatsel

@Shnatsel Shnatsel commented May 18, 2026

Copy link
Copy Markdown
Contributor

This is necessary for eventual AVX-512 support. Part of #179.

This does not add any AVX-512 stuff yet, just lays the groundwork by abstracting away the internal representation.

Contrary to what the description of #196 said, we don't actually need i64 vectors in the public API so long as we're not providing direct conversions between mask types and integer vector types, which aren't available even on main.

Summary of changes:

  • Add SimdMask<S> trait independent from SimdBase<S>.
  • Remove integer-vector-style APIs from masks:
    • Deref
    • Indexing
    • Bytes
    • public SimdSplit / SimdCombine
    • public slide / slide_within_blocks
    • public byte conversions
  • Remove scalar bit-op overloads for masks, so masks support mask-to-mask & | ^ ! but not mask & -1.

This was extricated from a larger changeset I had locally that also added some std::simd API compatibility functions, but that was getting too complicated to review. I'm happy to add more APIs if there's desire and review capacity for them.

A port of vello to this API can be found here.

@Shnatsel Shnatsel force-pushed the opaque-mask-representation-minimal branch from daee6e0 to 8dead9c Compare May 18, 2026 21:00
@Shnatsel

Copy link
Copy Markdown
Contributor Author

The test failure from a completely unrelated part is spooky. I checked it for UB under miri with various CPU feature combinations, but it passes there, as well as on real hardware (locally and in CI) and on Intel's emulator on CI.

I'm pinning it on a Rosetta bug, since real hardwware and 2 other emulators all pass.

When running x86 code on the macos-latest image, it runs through Rosetta. We need to switch over to macos-26-intel runner for x86 macos to get real x86 hardware.

@Shnatsel Shnatsel marked this pull request as ready for review May 19, 2026 00:54
@Shnatsel

Copy link
Copy Markdown
Contributor Author

Porting vello was trivial: https://github.com/linebender/vello/compare/main...Shnatsel:opaque-masks?expand=1

So this is ready for review.

@LaurenzV LaurenzV self-requested a review May 19, 2026 05:36

@LaurenzV LaurenzV left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't finished looking through it, just some first comments.

Comment on lines 893 to 895
fn as_array_mask8x16(self, a: mask8x16<Self>) -> [i8; 16usize] {
unsafe { core::mem::transmute::<__m128i, [i8; 16usize]>(a.val.0) }
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can we still keep this method? Wouldn't this also expect that the inner representation is backed by actual integers?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And similarly, how would load_array_mask8x16 be implemented for AVX512 if it takes the array as an integer array?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't keep Deref into an integer slice/array, but explicit conversion into an owned instance is perfectly fine and still useful. So conversions to/from the integer array representation are left intact in the API on purpose. They're the cheapest way to create a mask right now, and are decently cheap even on AVX-512 (a single instruction).

@@ -392,36 +396,12 @@ pub trait Simd:
fn widen_u8x16(self, a: u8x16<Self>) -> u16x16<Self>;
#[doc = "Reinterpret the bits of this vector as a vector of `u32` elements.\n\nThe total bit width is preserved; the number of elements changes accordingly."]
fn reinterpret_u32_u8x16(self, a: u8x16<Self>) -> u32x4<Self>;
#[doc = "Create a SIMD vector with all elements set to the given value."]
#[doc = "Create a SIMD mask with all lanes set from the given signed integer mask value."]
fn splat_mask8x16(self, val: i8) -> mask8x16<Self>;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would splat_mask be implemented for AVX-512 since the representation of a single entry is always just a single bit?

I guess more fundamentally, the question is how we fundamentally want to allow construction of masks while preventing hidden footguns due to the exact internals of the SIMD level. For example, I imagine a from_bitmask method would be fast for AVX-512 since its the native representation but slow on NEON since you need to expand the bitmask manually, while a from_array, as it is now, is the best for NEON but would require manually encoding the mask into a single bitmask. I'm not sure what the best thing to do is here, but probably we can take inspiration from portable SIMD?

@Shnatsel Shnatsel May 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would check if the value is 0, and treat any other value as 1.

I've actually started this work by taking inspiration from std::simd, and I have a local branch that is a much more complete mirror of std::simd APIs. This PR is a stripped down, MVP version of it.

std::simd only implements construction of masks from bits represented as a u64 and from arrays of booleans; not from [i32; 4]. Their trade-off is greater portability but worse performance. I implemented that at first, then rolled back the removal of integer-based APIs because the assembly for bool arrays looked gnarly.

Now that you point it out, I agree it's probably better to mirror std::simd and make splat operate on booleans. I guess writing == 0 is not too much to ask of the users. I'll make that change to better align with std::simd in this case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've experimented with this and looked at the assembly; it does add a neg instruction in the hot path for non-constant inputs on x86, so it's technically less efficient, but that's probably worth it for the nicer API.

Comment thread fearless_simd_gen/src/mk_simd_trait.rs Outdated
Shnatsel added 6 commits May 19, 2026 23:28
…g std::simd API. Adds a `neg` on the hot path on x86 for non-constant inputs, but seems cheap enough and worth it for the nicer API.
… actual integer values involved, now that we have boolean APIs and the conversions are relevant
…re not clashing with std::simd API of the same name
@Shnatsel

Copy link
Copy Markdown
Contributor Author

I'd like to add to_bitmask()/from_bitmask() as well as test and set to match std:simd, but I'll do it in a follow-up PR to avoid complicating review of this one.

@LaurenzV

Copy link
Copy Markdown
Collaborator

Will try to review the rest after RustWeek!

@LaurenzV LaurenzV left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I haven't tried to understand the new codegen logic very thoroughly, but the new generated code seems good to me and I agree that making the mask representation opaque is the right move towards getting AVX-512 working. It's a bit unfortunate that there is now more code duplication in the codegen code, but not a big deal.

"A SIMD mask of {len} {scalar_bits}-bit elements.\n\n\
When created from a comparison operation, and as it should be used in a [`Self::select`] operation, each element will be all ones if it's \"true\", and all zeroes if it's \"false\".",
"A SIMD mask of {len} logical lanes corresponding to {scalar_bits}-bit vector elements.\n\n\
The storage representation of this type is intentionally opaque. For compatibility with existing APIs, it may be converted to and from signed integer lanes where false is encoded as all zeroes (integer value 0) and true is encoded as all ones (integer value -1).",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add a note that the conversion to/from signed integers might not be zero-cost for all levels though?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll describe the efficiency in a follow-up where I add other conversion paths

"Behavior on mask elements that are not all zeroes or all ones is unspecified. It may vary depending on architecture, feature level, the mask elements' width, the mask vector's width, or library version.\n\n\
The behavior is also not guaranteed to be logically consistent if mask elements are not all zeroes or all ones. `any_true` may not return the same result as `!all_false`, and `all_true` may not return the same result as `!any_false`.\n\n\
The [`select`](crate::Select::select) operation also has unspecified behavior for mask elements that are not all zeroes or all ones. That behavior may not match the behavior of this operation."
"Masks may be converted to and from signed integer lane arrays for compatibility with older APIs. For those conversions, false is encoded as all zeroes (integer value 0) and true is encoded as all ones (integer value -1).\n\n\

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@LaurenzV

Copy link
Copy Markdown
Collaborator

It seems like you might have to rebase or open a new PR to make CI work correctly again.

Shnatsel added a commit to Shnatsel/fearless_simd that referenced this pull request May 23, 2026
`macos-latest` is ARM, and runs x86 binaries through rosetta. We need to
specify the Intel runner explicitly to get x86 hardware.

Motivation:
linebender#218 (comment)

Our test suite is now so thorough that it caught a Rosetta bug
:sunglasses:
@Shnatsel Shnatsel added this pull request to the merge queue May 23, 2026
@Shnatsel

Copy link
Copy Markdown
Contributor Author

I've reduced the duplication in the generation of splat().

There is more refactoring we could do, but I haven't found the right cut points for shared logic yet. The conditions end up being more awkward and hard to follow when they're not defined at the leaf.

Merged via the queue into linebender:main with commit a1f5cd2 May 23, 2026
22 checks passed
@Shnatsel Shnatsel deleted the opaque-mask-representation-minimal branch May 23, 2026 10:51
LaurenzV pushed a commit to LaurenzV/fearless_simd that referenced this pull request Jun 27, 2026
Follow-up to linebender#218

Adds to/from_bitmask, mirroring `std::simd`. This required substantial
complexity, and is covered with exhaustive roundtrip tests for smaller
sizes and tests for interesting patterns on larger sizes.

Also re-introduces APIs to get/set a single bit. The API mirrors
`std::simd` instead of the previous `Index` trait. The implementation
has minimal complexity; it reuses `to_bitmask()`, which is why both
changes are in the same PR.

Not included in this PR: better docs on mask types, including the cost
trade-offs for various ways of creating them. I'd like to add that in a
follow-up.

## Performance

### from_bitmask

to/from_bitmask are lowered to intrinsics for each backend.

Our codegen for `from_bitmask()` is on par with or better than std::simd
on both x86 and NEON. Making it fast on NEON was quite easy -
`std::simd` performs scalar bit extraction for many widths, which is
slow, so on NEON we often win by a landslide. It's so bad that I
[reported this
upstream](https://rust-lang.zulipchat.com/#narrow/channel/257879-project-portable-simd/topic/poor.20codegen.20for.20Mask.3A.3Afrom_bitmask.28.29.20on.20NEON).
x86 required more effort for parity, but we're on-par-or-better there
too, dramatically so for some sizes.

<details><summary>from_bitmask performance vs std::simd</summary>
<p>

`from_bitmask` sweep with `llvm-mca-14 -mcpu=znver3` (-mcpu=neoverse-n1
for NEON), commit 9b7a6c5 vs nightly
`std::simd` on `1.97.0-nightly (14196dbfa 2026-04-12)`

x86 numbers sanity-checked with `perf` on zen4 and line up with the
simulation

**NEON**

| mask | uops ours/std | RT ours/std | read |
|---|---:|---:|---|
| `mask8x16` | `20/55` | `6.7/18.3` | ours much better |
| `mask16x8` | `11/31` | `3.7/10.3` | ours much better |
| `mask32x4` | `11/22` | `3.7/7.3` | ours better |
| `mask64x2` | `11/16` | `3.7/5.3` | ours better |
| `mask8x32` | `37/102` | `12.3/34.0` | ours much better |
| `mask16x16` | `19/62` | `6.3/20.7` | ours much better |
| `mask32x8` | `19/38` | `6.3/12.7` | ours better |
| `mask64x4` | `19/26` | `6.3/8.7` | ours better |
| `mask8x64` | `72/204` | `24.0/68.0` | ours much better |
| `mask16x32` | `36/117` | `12.0/39.0` | ours much better |
| `mask32x16` | `36/71` | `12.0/23.7` | ours better |
| `mask64x8` | `36/47` | `12.0/15.7` | ours better |

**SSE4.2**
| mask | uops ours/std | RT ours/std | read |
|---|---:|---:|---|
| `mask8x16` | 7/7 | 2.5/2.5 | tie |
| `mask16x8` | 8/8 | 2.0/2.0 | tie |
| `mask32x4` | 7/25 | 2.0/4.5 | ours much better |
| `mask64x2` | 7/14 | 2.0/2.5 | ours better |
| `mask8x32` | 12/14 | 3.5/2.5 | mixed, std RT better |
| `mask16x16` | 12/13 | 3.0/3.0 | ours slight |
| `mask32x8` | 12/12 | 2.5/3.0 | ours slight |
| `mask64x4` | 13/56 | 3.5/9.3 | ours much better |
| `mask8x64` | 24/24 | 4.5/4.5 | tie |
| `mask16x32` | 22/22 | 5.0/4.0 | std RT better |
| `mask32x16` | 22/22 | 4.0/5.0 | ours RT better |
| `mask64x8` | 23/22 | 4.5/5.0 | mixed, ours RT better |

**AVX2**
| mask | uops ours/std | RT ours/std | read |
|---|---:|---:|---|
| `mask8x16` | 7/7 | 2.5/2.5 | tie |
| `mask16x8` | 7/7 | 2.0/2.0 | tie |
| `mask32x4` | 7/21 | 2.0/4.5 | ours much better |
| `mask64x2` | 7/12 | 2.0/2.5 | ours better |
| `mask8x32` | 9/9 | 2.5/2.5 | tie |
| `mask16x16` | 8/8 | 2.0/2.0 | tie |
| `mask32x8` | 8/8 | 2.0/2.0 | tie |
| `mask64x4` | 8/24 | 2.0/4.5 | ours much better |
| `mask8x64` | 13/13 | 3.5/3.5 | tie |
| `mask16x32` | 14/15 | 3.5/3.5 | ours slight |
| `mask32x16` | 12/12 | 3.0/3.0 | tie |
| `mask64x8` | 12/12 | 3.0/3.0 | tie |

</p>
</details> 

## to_bitmask
`to_bitmask()` was mostly straightforward, but required special handling
for 16-bit masks on x86. We're on par or better with `std::simd`, except
for the `mask16x32` case on AVX2, where std gets the ideal
`vpacksswb`-style lowering and we only get `vpmovmskb` + `pext`. I
couldn't get rustc to emit `vpacksswb`.

<details><summary>to_bitmask performance vs std::simd</summary>
<p>

`to_bitmask` sweep with `llvm-mca-14 -mcpu=znver3` (-mcpu=neoverse-n1
for NEON), commit 9b7a6c5 vs nightly
`std::simd` on `1.97.0-nightly (14196dbfa 2026-04-12)`

x86 numbers sanity-checked with `perf` on zen4 and line up with the
simulation

**NEON**

| mask | uops ours/std | RT ours/std | read |
|---|---:|---:|---|
| `mask8x16` | `12/14` | `4.0/4.7` | ours better |
| `mask16x8` | `9/10` | `3.0/3.3` | ours slight |
| `mask32x4` | `8/13` | `3.0/4.3` | ours better |
| `mask64x2` | `7/15` | `3.0/5.0` | ours better |
| `mask8x32` | `23/25` | `7.7/8.3` | ours slight |
| `mask16x16` | `17/22` | `5.7/7.3` | ours better |
| `mask32x8` | `15/18` | `5.0/6.0` | ours better |
| `mask64x4` | `13/19` | `5.0/6.3` | ours better |
| `mask8x64` | `44/48` | `14.7/16.0` | ours slight |
| `mask16x32` | `32/38` | `10.7/12.7` | ours better |
| `mask32x16` | `28/33` | `9.3/11.0` | ours better |
| `mask64x8` | `24/29` | `9.0/9.7` | ours slight |

**SSE4.2**

| mask | uops ours/std | RT ours/std | read |
|---|---:|---:|---|
| `mask8x16` | `3/3` | `1.0/1.0` | tie |
| `mask16x8` | `5/5` | `1.0/1.0` | tie |
| `mask32x4` | `3/7` | `1.0/1.2` | ours better |
| `mask64x2` | `3/9` | `1.0/1.5` | ours better |
| `mask8x32` | `7/7` | `2.0/2.0` | tie |
| `mask16x16` | `4/4` | `1.0/1.0` | tie |
| `mask32x8` | `6/6` | `1.0/1.0` | tie |
| `mask64x4` | `4/11` | `1.0/1.8` | ours better |
| `mask8x64` | `15/15` | `4.0/4.0` | tie |
| `mask16x32` | `9/9` | `2.0/2.0` | tie |
| `mask32x16` | `7/7` | `2.0/2.0` | tie |
| `mask64x8` | `15/15` | `2.5/2.5` | tie |

**AVX2**

| mask | uops ours/std | RT ours/std | read |
|---|---:|---:|---|
| `mask8x16` | `3/3` | `1.0/1.0` | tie |
| `mask16x8` | `5/5` | `1.0/1.0` | tie |
| `mask32x4` | `3/4` | `1.0/1.0` | ours slight |
| `mask64x2` | `3/8` | `1.0/1.3` | ours better |
| `mask8x32` | `4/4` | `1.0/1.0` | tie |
| `mask16x16` | `4/4` | `1.0/1.0` | tie |
| `mask32x8` | `4/4` | `1.0/1.0` | tie |
| `mask64x4` | `4/8` | `1.0/1.3` | ours better |
| `mask8x64` | `8/8` | `2.0/2.0` | tie |
| `mask16x32` | `11/7` | `2.0/1.2` | std better |
| `mask32x16` | `10/10` | `1.7/1.7` | tie |
| `mask64x8` | `7/7` | `1.2/1.2` | tie |

</p>
</details> 

### set

test/set are implemented with generic codepaths without messing with
intrinsics, since they're less performance-critical and the generic
codegen is already good.

`set()` is implemented by converting the vector to an array and flipping
the one value with a scalar instruction. This matches `std::simd`
assembly. On x86 a much more complex path that keeps values in registers
is possible, but it's only 5-10% faster and doesn't seem to be worth the
effort. It can be added in a follow-up PR if desired.

### test

`test()` is implemented via `to_bitmask()` which deviates from the
assembly produced by std::simd. It is faster than roundtripping through
an array if your vector is already in registers, e.g. via
`a.simd_cmp(b).test(n)` (8 cycles instead of 12 on llvm-mca). However it
is slower if the vector is already spilled to the stack. I've chosen to
err on the side of not forcing stack spills. Reasonable people can
disagree about the trade-offs here, but I don't think it really matters
since this function is not performance-critical anyway.
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.

2 participants