Add support for unaligned simd masked load/store

[sayantn]

We currently have only an aligned version of simd_masked_load, but the underlying llvm intrinsic doesn't have any alignment requirement. So I propose to add simd_masked_load_unaligned and simd_masked_store_unaligned. These will link with the llvm intrinsic with an alignment of 8 bits (the lowest permissible in Rust)

[workingjubilee]

The alignment required is only the alignment of the T for the Simd<T, N>.

[workingjubilee]

Do you have a use-case in mind?

[workingjubilee]

Oh, I see. It seems this is about rust-lang/stdarch#1594 yes? Except you have underaligned loads and stores, also!

I think I'd rather see us pass the alignment as a const generic parameter if we want to go down this road, then we can pass 1 or whatever.

[sayantn]

Yes, this is for stdarch. There are a lot of unaligned masked load/store intrinsics, and they all resort to using inline asm.

[workingjubilee]

the aligned ones also, thus my remark about a generic alignment. (at least, I hope emitting the correct alignment would encourage LLVM to lower things correctly...)

[sayantn]

Even if we add a generic alignment parameter, most if not all use cases will be fully-aligned or fully-unaligned. Also, having a random ::<1> in the middle of simd code will be somewhat confusing

[workingjubilee]

For who? Standard library developers?

I agree it doesn't have many possible values though.

[sayantn]

For any contributor, it may be confusing. I think stdarch code should be as clear as possible to avoid possible bugs

[sayantn]

The only practical values are 1, 2, 4 and 8

[workingjubilee]

The only practical values are 1, 2, 4 and 8

Not if you want to emit vmovaps. That puts 16, 32, and 64 on the table.

And I understand that desire, but I also think that the compiler code implementing these intrinsics should be as clear as possible to avoid possible bugs.

[sayantn]

Oh, yeah. I misread the doc. Then it should be a const generic parameter, as it has quite a few possible values (1 for movu, 16/32/64 for mova, 1/2/4/8/16 for portable simd)

[sayantn]

Should we introduce a new function with an alignment const generic parameter, or should we modify the existing masked load/store functions to take an extra const generic parameter (breaking change)?

[workingjubilee]

I would prefer the latter if possible.

[sayantn]

It is possible to do, we just need to grep stdlib for its uses (I don't think there are any uses except for portable simd). In portable simd, we need something like simd_masked_load::<{ mem:: align_of::<T> }>(mask, ptr, val), this just needs #[feature(generic_const_exprs)]. Even outside of portable simd, there should not be many uses of these, since they were added only last December.

[workingjubilee]

oh, darn.
we can't rely on generic_const_exprs, sadly.

[sayantn]

We can also use inline consts and a normal alignment parameter, like used in simd_shuffle as a shuffle parameter

[RalfJung]

We can also use inline consts and a normal alignment parameter, like used in simd_shuffle as a shuffle parameter

That's an ad-hoc hack and something we'd rather not see more of.

[sayantn]

The alternative, using const generics, will need generic_const_exprs in portable-simd.

[RalfJung]

Yeah it may just currently not be possible to do this, then.

Why does alignment need to be a const parameter anyway? Usually in Rust the alignment is given by the type.

[sayantn]

llvm needs it to be an immediate value. the alignment is for the pointer. if the pointer is 16-byte aligned and we are loading a 128-bit value, llvm can generate an aligned load, but not if it is not 16-byte aligned. in this case the element type may be u8 or whatever

[RalfJung]

Elsewhere in Rust people encode that with types just fine, e.g. via #[repr(align(N))]. I don't think I understand why it does not work here.

[sayantn]

by default, these intrinsics use the alignment of the element type. and that may not be the desired alignment of the pointer. we may have a u8x16, then the intrinsic will assume that the pointer is byte-aligned. but for aligned simd loads, the pointer is 128-bit aligned. even bad scenario, we need to load from a byte-aligned pointer into a u32x4. it is not even possible using this intrinsic

[RalfJung]

So should the definition of u8x16 be changed to require 16-byte alignment?

even bad scenario, we need to load from a byte-aligned pointer into a u32x4. it is not even possible using this intrinsic

We have read_unaligned for that. You'll need a raw pointer here anyway, unaligned references are UB (even if you never load from them).

[RalfJung]

Ah wait this is about masked access, so I guess read_unaligned won't work?

In general Rust doesn't support having over/underaligned references. It's either "use standard alignment" or "use raw pointer". There's clearly a gap here, supporting explicit alignment annotations would be nice, but I don't think it makes sense for the SIMD types to be the part that tries to work out a solution -- this needs a language-side solution.

Yes, this is for stdarch. There are a lot of unaligned masked load/store intrinsics, and they all resort to using inline asm.

Until const generics get better, using inline asm seems like not the worst solution to me...

[sayantn]

We don't need to do much, the LLVM intrinsic already has an alignment parameter. For masked access, a reference doesn't really make sense, as the point of masked loads, instead of load-then-select is that if you don't have permission to load from the masked-out positions, it should work just fine. And a reference is always safe to dereference, in which case we can just use load-then-select

[RalfJung]

Yeah sorry I had forgotten that this was about masked accesses.

We don't need to do much,

Not much, just finish const generics? ;)

You are asking for quite a lot here. Compile-time constants passed as parameters to functions are a non-trivial topic. So "don't need to do much" is not accurate I am afraid.

[workingjubilee]

Hmm, I somewhat wonder if the proper direction would be supporting more ergonomic ways to specify the alignment of types...

[sayantn]

Why does portable_simd even use the alignment of the element type? It is an awkward position between traditionally "aligned" (to the vector type) and "unaligned" (byte-aligned) simd load/stores. We can avoid the problem of const generics simply by modifying the existing intrinsic to use the alignment of the vector type, and adding a new intrinsic that generates byte-aligned load/stores. Or, we can add a const generic boolean ALIGNED parameter.

[RalfJung]

How did this issue get resolved?

[sayantn]

This is no longer needed in stdarch because we used some llvm intrinsics directly, and as this issue was opened because of stdarch I thought we don't need it anymore

[RalfJung]

Would be good to cross-reference that change with this issue, so that in the future someone can figure out what happened.

[sayantn]

rust-lang/stdarch#1613 eliminated the need for this feature in strarch

[RalfJung]

Thanks :)