Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use Unsafe.ReadUnaligned for vector creation in SpanHelpers.Fill #111091

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+8 −8
Open

Use Unsafe.ReadUnaligned for vector creation in SpanHelpers.Fill #111091

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
93e46f0
Use `Unsafe.BitCast` to avoid taking address
xtqqczze Jan 4, 2025
89f0c1f
Revert "Use `Unsafe.BitCast` to avoid taking address"
xtqqczze Jan 5, 2025
4973eed
fixes
xtqqczze Jan 5, 2025
8655472
Merge branch 'main' into use-unsafe-bitcast
xtqqczze Feb 5, 2025
c43839b
Merge branch 'main' into use-unsafe-bitcast
xtqqczze Feb 11, 2025
d7fb33a
Merge branch 'main' into use-unsafe-bitcast
xtqqczze Feb 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,29 @@ public static unsafe void Fill<T>(ref T refData, nuint numElements, T value)

if (sizeof(T) == 1)
{
vector = new Vector<byte>(Unsafe.As<T, byte>(ref tmp));
vector = Vector.Create(Unsafe.As<T, byte>(ref tmp));
Copy link
Member

Choose a reason for hiding this comment

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

these should rather use Vector.LoadUnsafe(ref tmp).AsByte() or similar.

We should be explicit that its a load API and not do extra or other things that the JIT has to elide or reason about

Copy link
Member

Choose a reason for hiding this comment

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

Ah nevermind, I see the issue here. This is doing a broadcast and is reinterpreting the ref to a supported T

This pattern in general is not great and something that we probably should have a centralized helper or some other API around...

}
else if (sizeof(T) == 2)
{
vector = (Vector<byte>)(new Vector<ushort>(Unsafe.As<T, ushort>(ref tmp)));
vector = (Vector<byte>)Vector.Create(Unsafe.ReadUnaligned<ushort>(ref Unsafe.As<T, byte>(ref tmp)));
}
else if (sizeof(T) == 4)
{
// special-case float since it's already passed in a SIMD reg
vector = (typeof(T) == typeof(float))
? (Vector<byte>)(new Vector<float>((float)(object)tmp!))
: (Vector<byte>)(new Vector<uint>(Unsafe.As<T, uint>(ref tmp)));
? (Vector<byte>)Vector.Create(Unsafe.BitCast<T, float>(tmp))
: (Vector<byte>)Vector.Create(Unsafe.ReadUnaligned<uint>(ref Unsafe.As<T, byte>(ref tmp)));
}
else if (sizeof(T) == 8)
{
// special-case double since it's already passed in a SIMD reg
vector = (typeof(T) == typeof(double))
? (Vector<byte>)(new Vector<double>((double)(object)tmp!))
: (Vector<byte>)(new Vector<ulong>(Unsafe.As<T, ulong>(ref tmp)));
? (Vector<byte>)Vector.Create(Unsafe.BitCast<T, double>(tmp))
: (Vector<byte>)Vector.Create(Unsafe.ReadUnaligned<ulong>(ref Unsafe.As<T, byte>(ref tmp)));
}
else if (sizeof(T) == 16)
{
Vector128<byte> vec128 = Unsafe.As<T, Vector128<byte>>(ref tmp);
Vector128<byte> vec128 = Vector128.LoadUnsafe(ref Unsafe.As<T, byte>(ref tmp));
if (Vector<byte>.Count == 16)
{
vector = vec128.AsVector();
Expand All @@ -75,7 +75,7 @@ public static unsafe void Fill<T>(ref T refData, nuint numElements, T value)
{
if (Vector<byte>.Count == 32)
{
vector = Unsafe.As<T, Vector256<byte>>(ref tmp).AsVector();
vector = Vector256.LoadUnsafe(ref Unsafe.As<T, byte>(ref tmp)).AsVector();
}
else
{
Expand Down
Loading