Skip to content

Update a few code paths to ensure the trimmer can do its job #106777

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 19, 2024
Merged

Update a few code paths to ensure the trimmer can do its job #106777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b0ff86
Update a few code paths to ensure the trimmer can do its job
tannergooding Aug 21, 2024
1d6ad1b
Ensure that the throw helper works on downlevel targets
tannergooding Aug 22, 2024
0477988
Merge branch 'main' into fix-93399
tannergooding Oct 18, 2024
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
8 changes: 7 additions & 1 deletion src/libraries/Common/src/System/HexConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,14 +292,20 @@ public static bool TryDecodeFromUtf16_Vector128(ReadOnlySpan<char> chars, Span<b
output = Ssse3.MultiplyAddAdjacent(nibbles,
Vector128.Create((short)0x0110).AsSByte()).AsByte();
}
else
else if (AdvSimd.Arm64.IsSupported)
{
// Workaround for missing MultiplyAddAdjacent on ARM
Vector128<short> even = AdvSimd.Arm64.TransposeEven(nibbles, Vector128<byte>.Zero).AsInt16();
Vector128<short> odd = AdvSimd.Arm64.TransposeOdd(nibbles, Vector128<byte>.Zero).AsInt16();
even = AdvSimd.ShiftLeftLogical(even, 4).AsInt16();
output = AdvSimd.AddSaturate(even, odd).AsByte();
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
Copy link
Member

@eiriktsarpalis eiriktsarpalis Oct 18, 2024

Choose a reason for hiding this comment

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

Could you explain why this added branch is necessary? My expectation is that the entire method should have been trimmed altogether if its preconditions are not met.

Copy link
Member Author

@tannergooding tannergooding Oct 18, 2024

Choose a reason for hiding this comment

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

Some of the checks are "dynamic" and not statically true/false.

For example, if (Sse2.IsSupported) is statically true for x86/x64 and if (AdvSimd.Arm64.IsSupported) is statically false, but if (Ssse3.IsSupported) is dynamic because it depends on the underlying hardware.

This means that on an x86/x64 system, the TryDecodeFromUtf16 will always be preserved in IL. Prior to this PR, the else path was therefore being kept for the case that we were on hardware where Ssse3.IsSupported was false and that rooted AdvSimd.Arm64 on xarch unnecessarily.

With this change, we instead now allow the AdvSimd path to be trimmed out as dead code and the IL will instead keep the ThrowUnreachableException which allows things like R2R, the JIT, or AOT compilers to trim it out as dead code (because they will know the target hardware and treat Ssse3.IsSupported as constant, then allowing the method to be removed).

Copy link
Member

@eiriktsarpalis eiriktsarpalis Oct 18, 2024

Choose a reason for hiding this comment

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

Thanks

ThrowHelper.ThrowUnreachableException();
output = default;
}
// Accumulate output in lower INT64 half and take care about endianness
output = Vector128.Shuffle(output, Vector128.Create((byte)0, 2, 4, 6, 8, 10, 12, 14, 0, 0, 0, 0, 0, 0, 0, 0));
// Store 8 bytes in dest by given offset
Expand Down
16 changes: 14 additions & 2 deletions ...raries/System.Private.CoreLib/src/System/Buffers/Text/Base64Helper/Base64DecoderHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1105,12 +1105,18 @@ private static unsafe void Vector128Decode<TBase64Decoder, T>(TBase64Decoder dec
{
merge_ab_and_bc = Ssse3.MultiplyAddAdjacent(str.AsByte(), mergeConstant0.AsSByte());
}
else
else if (AdvSimd.Arm64.IsSupported)
{
Vector128<ushort> evens = AdvSimd.ShiftLeftLogicalWideningLower(AdvSimd.Arm64.UnzipEven(str, one).GetLower(), 6);
Vector128<ushort> odds = AdvSimd.Arm64.TransposeOdd(str, Vector128<byte>.Zero).AsUInt16();
merge_ab_and_bc = Vector128.Add(evens, odds).AsInt16();
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowUnreachableException();
merge_ab_and_bc = default;
}
// 0000kkkk LLllllll 0000JJJJ JJjjKKKK
// 0000hhhh IIiiiiii 0000GGGG GGggHHHH
// 0000eeee FFffffff 0000DDDD DDddEEEE
Expand All @@ -1121,12 +1127,18 @@ private static unsafe void Vector128Decode<TBase64Decoder, T>(TBase64Decoder dec
{
output = Sse2.MultiplyAddAdjacent(merge_ab_and_bc, mergeConstant1);
}
else
else if (AdvSimd.Arm64.IsSupported)
{
Vector128<int> ievens = AdvSimd.ShiftLeftLogicalWideningLower(AdvSimd.Arm64.UnzipEven(merge_ab_and_bc, one.AsInt16()).GetLower(), 12);
Vector128<int> iodds = AdvSimd.Arm64.TransposeOdd(merge_ab_and_bc, Vector128<short>.Zero).AsInt32();
output = Vector128.Add(ievens, iodds).AsInt32();
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowUnreachableException();
output = default;
}
// 00000000 JJJJJJjj KKKKkkkk LLllllll
// 00000000 GGGGGGgg HHHHhhhh IIiiiiii
// 00000000 DDDDDDdd EEEEeeee FFffffff
Expand Down
16 changes: 14 additions & 2 deletions ...raries/System.Private.CoreLib/src/System/Buffers/Text/Base64Helper/Base64EncoderHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,12 +505,18 @@ private static unsafe void Vector128Encode<TBase64Encoder, T>(TBase64Encoder enc
{
t1 = Sse2.MultiplyHigh(t0.AsUInt16(), shiftAC);
}
else
else if (AdvSimd.Arm64.IsSupported)
{
Vector128<ushort> odd = Vector128.ShiftRightLogical(AdvSimd.Arm64.UnzipOdd(t0.AsUInt16(), t0.AsUInt16()), 6);
Vector128<ushort> even = Vector128.ShiftRightLogical(AdvSimd.Arm64.UnzipEven(t0.AsUInt16(), t0.AsUInt16()), 10);
t1 = AdvSimd.Arm64.ZipLow(even, odd);
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowUnreachableException();
t1 = default;
}
// 00000000 00kkkkLL 00000000 00JJJJJJ
// 00000000 00hhhhII 00000000 00GGGGGG
// 00000000 00eeeeFF 00000000 00DDDDDD
Expand Down Expand Up @@ -545,10 +551,16 @@ private static unsafe void Vector128Encode<TBase64Encoder, T>(TBase64Encoder enc
{
indices = Sse2.SubtractSaturate(str.AsByte(), const51);
}
else
else if (AdvSimd.IsSupported)
{
indices = AdvSimd.SubtractSaturate(str.AsByte(), const51);
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowUnreachableException();
indices = default;
}

// mask is 0xFF (-1) for range #[1..4] and 0x00 for range #0:
Vector128<sbyte> mask = Vector128.GreaterThan(str.AsSByte(), const25);
Expand Down
11 changes: 11 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Base64Helper/Base64Helper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
#if NET
using System.Runtime.Intrinsics;
Expand Down Expand Up @@ -181,6 +182,16 @@ internal static Vector128<byte> ShuffleUnsafe(Vector128<byte> vector, Vector128<
}
#endif

[DoesNotReturn]
internal static void ThrowUnreachableException()
{
#if NET
throw new UnreachableException();
Copy link
Member

@eiriktsarpalis eiriktsarpalis Oct 18, 2024

Choose a reason for hiding this comment

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

TIL that this exception type exists. Does it have any special meaning from the perspective of the trimmer?

Copy link
Member Author

@tannergooding tannergooding Oct 18, 2024

Choose a reason for hiding this comment

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

No, its just the exception type we added explicitly to represent paths that should be "unreachable", so its the most appropriate one to use here.

#else
throw new Exception("Unreachable");
#endif
}

internal interface IBase64Encoder<T> where T : unmanaged
{
ReadOnlySpan<byte> EncodingMap { get; }
Expand Down
39 changes: 32 additions & 7 deletions src/libraries/System.Private.CoreLib/src/System/SearchValues/IndexOfAnyAsciiSearcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1368,9 +1368,20 @@ public static Vector128<byte> PackSources(Vector128<ushort> lower, Vector128<ush
Vector128<short> lowerMin = Vector128.Min(lower, Vector128.Create((ushort)255)).AsInt16();
Vector128<short> upperMin = Vector128.Min(upper, Vector128.Create((ushort)255)).AsInt16();

return Sse2.IsSupported
? Sse2.PackUnsignedSaturate(lowerMin, upperMin)
: PackedSimd.ConvertNarrowingSaturateUnsigned(lowerMin, upperMin);
if (Sse2.IsSupported)
{
return Sse2.PackUnsignedSaturate(lowerMin, upperMin);
}
else if (PackedSimd.IsSupported)
{
return PackedSimd.ConvertNarrowingSaturateUnsigned(lowerMin, upperMin);
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowHelper.ThrowUnreachableException();
return default;
}
}

// Replace with Vector256.NarrowWithSaturation once https://github.com/dotnet/runtime/issues/75724 is implemented.
Expand All @@ -1392,10 +1403,24 @@ public static Vector256<byte> PackSources(Vector256<ushort> lower, Vector256<ush
[CompExactlyDependsOn(typeof(PackedSimd))]
public static Vector128<byte> PackSources(Vector128<ushort> lower, Vector128<ushort> upper)
{
return
Sse2.IsSupported ? Sse2.PackUnsignedSaturate(lower.AsInt16(), upper.AsInt16()) :
AdvSimd.IsSupported ? AdvSimd.ExtractNarrowingSaturateUpper(AdvSimd.ExtractNarrowingSaturateLower(lower), upper) :
PackedSimd.ConvertNarrowingSaturateUnsigned(lower.AsInt16(), upper.AsInt16());
if (Sse2.IsSupported)
{
return Sse2.PackUnsignedSaturate(lower.AsInt16(), upper.AsInt16());
}
else if (AdvSimd.IsSupported)
{
return AdvSimd.ExtractNarrowingSaturateUpper(AdvSimd.ExtractNarrowingSaturateLower(lower), upper);
}
else if (PackedSimd.IsSupported)
{
return PackedSimd.ConvertNarrowingSaturateUnsigned(lower.AsInt16(), upper.AsInt16());
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowHelper.ThrowUnreachableException();
return default;
}
}

[CompExactlyDependsOn(typeof(Avx2))]
Expand Down
26 changes: 20 additions & 6 deletions src/libraries/System.Private.CoreLib/src/System/SearchValues/ProbabilisticMap.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,13 +219,27 @@ private static Vector128<byte> ContainsMask16Chars(Vector128<byte> charMapLower,
Vector128<ushort> source0 = Vector128.LoadUnsafe(ref searchSpace);
Vector128<ushort> source1 = Vector128.LoadUnsafe(ref searchSpace, (nuint)Vector128<ushort>.Count);

Vector128<byte> sourceLower = Sse2.IsSupported
? Sse2.PackUnsignedSaturate((source0 & Vector128.Create((ushort)255)).AsInt16(), (source1 & Vector128.Create((ushort)255)).AsInt16())
: AdvSimd.Arm64.UnzipEven(source0.AsByte(), source1.AsByte());
Vector128<byte> sourceLower;
Vector128<byte> sourceUpper;

Vector128<byte> sourceUpper = Sse2.IsSupported
? Sse2.PackUnsignedSaturate((source0 >>> 8).AsInt16(), (source1 >>> 8).AsInt16())
: AdvSimd.Arm64.UnzipOdd(source0.AsByte(), source1.AsByte());
if (Sse2.IsSupported)
{
sourceLower = Sse2.PackUnsignedSaturate((source0 & Vector128.Create((ushort)255)).AsInt16(), (source1 & Vector128.Create((ushort)255)).AsInt16());
sourceUpper = Sse2.PackUnsignedSaturate((source0 >>> 8).AsInt16(), (source1 >>> 8).AsInt16());
}
else if (AdvSimd.Arm64.IsSupported)
{
sourceLower = AdvSimd.Arm64.UnzipEven(source0.AsByte(), source1.AsByte());
sourceUpper = AdvSimd.Arm64.UnzipOdd(source0.AsByte(), source1.AsByte());
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowHelper.ThrowUnreachableException();

sourceLower = default;
sourceUpper = default;
}

Vector128<byte> resultLower = IsCharBitNotSet(charMapLower, charMapUpper, sourceLower);
Vector128<byte> resultUpper = IsCharBitNotSet(charMapLower, charMapUpper, sourceUpper);
Expand Down
51 changes: 42 additions & 9 deletions src/libraries/System.Private.CoreLib/src/System/SearchValues/Strings/Helpers/TeddyHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,20 @@ public static Vector128<byte> LoadAndPack16AsciiChars(ref char source)
Vector128<ushort> source0 = Vector128.LoadUnsafe(ref source);
Vector128<ushort> source1 = Vector128.LoadUnsafe(ref source, (nuint)Vector128<ushort>.Count);

return Sse2.IsSupported
? Sse2.PackUnsignedSaturate(source0.AsInt16(), source1.AsInt16())
: AdvSimd.ExtractNarrowingSaturateUpper(AdvSimd.ExtractNarrowingSaturateLower(source0), source1);
if (Sse2.IsSupported)
{
return Sse2.PackUnsignedSaturate(source0.AsInt16(), source1.AsInt16());
}
else if (AdvSimd.IsSupported)
{
return AdvSimd.ExtractNarrowingSaturateUpper(AdvSimd.ExtractNarrowingSaturateLower(source0), source1);
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowHelper.ThrowUnreachableException();
return default;
}
}

// Read two Vector512<ushort> and concatenate their lower bytes together into a single Vector512<byte>.
Expand Down Expand Up @@ -323,9 +334,20 @@ private static Vector128<byte> RightShift1(Vector128<byte> left, Vector128<byte>
// We want to shift the last element of left (15) to be the first element of the result
// result: [15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30]

return Ssse3.IsSupported
? Ssse3.AlignRight(right, left, 15)
: AdvSimd.ExtractVector128(left, right, 15);
if (Ssse3.IsSupported)
{
return Ssse3.AlignRight(right, left, 15);
}
else if (AdvSimd.IsSupported)
{
return AdvSimd.ExtractVector128(left, right, 15);
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowHelper.ThrowUnreachableException();
return default;
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand All @@ -339,9 +361,20 @@ private static Vector128<byte> RightShift2(Vector128<byte> left, Vector128<byte>
// We want to shift the last two elements of left (14, 15) to be the first elements of the result
// result: [14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29]

return Ssse3.IsSupported
? Ssse3.AlignRight(right, left, 14)
: AdvSimd.ExtractVector128(left, right, 14);
if (Ssse3.IsSupported)
{
return Ssse3.AlignRight(right, left, 14);
}
else if (AdvSimd.IsSupported)
{
return AdvSimd.ExtractVector128(left, right, 14);
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowHelper.ThrowUnreachableException();
return default;
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand Down
21 changes: 18 additions & 3 deletions src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf8Utility.Transcoding.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,7 @@ public static OperationStatus TranscodeToUtf8(char* pInputBuffer, int inputLengt
Vector64<byte> lower = AdvSimd.ExtractNarrowingSaturateUnsignedLower(utf16Data);
AdvSimd.Store(pOutputBuffer, lower);
}
else
else if (Sse41.IsSupported)
{
if (!Sse41.TestZ(utf16Data, nonAsciiUtf16DataMask))
{
Expand All @@ -976,6 +976,11 @@ public static OperationStatus TranscodeToUtf8(char* pInputBuffer, int inputLengt
// narrow and write
Sse2.StoreScalar((ulong*)pOutputBuffer /* unaligned */, Sse2.PackUnsignedSaturate(utf16Data, utf16Data).AsUInt64());
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowHelper.ThrowUnreachableException();
}

pInputBuffer += 8;
pOutputBuffer += 8;
Expand All @@ -1000,10 +1005,15 @@ public static OperationStatus TranscodeToUtf8(char* pInputBuffer, int inputLengt
Vector64<byte> lower = AdvSimd.ExtractNarrowingSaturateUnsignedLower(utf16Data);
AdvSimd.StoreSelectedScalar((uint*)pOutputBuffer, lower.AsUInt32(), 0);
}
else
else if (Sse2.IsSupported)
{
Unsafe.WriteUnaligned(pOutputBuffer, Sse2.ConvertToUInt32(Sse2.PackUnsignedSaturate(utf16Data, utf16Data).AsUInt32()));
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowHelper.ThrowUnreachableException();
}

pInputBuffer += 4;
pOutputBuffer += 4;
Expand Down Expand Up @@ -1038,10 +1048,15 @@ public static OperationStatus TranscodeToUtf8(char* pInputBuffer, int inputLengt
Vector64<byte> lower = AdvSimd.ExtractNarrowingSaturateUnsignedLower(utf16Data);
AdvSimd.StoreSelectedScalar((uint*)pOutputBuffer, lower.AsUInt32(), 0);
}
else
else if (Sse2.IsSupported)
{
Unsafe.WriteUnaligned(pOutputBuffer, Sse2.ConvertToUInt32(Sse2.PackUnsignedSaturate(utf16Data, utf16Data).AsUInt32()));
}
else
{
// We explicitly recheck each IsSupported query to ensure that the trimmer can see which paths are live/dead
ThrowHelper.ThrowUnreachableException();
}
pInputBuffer += 4;
pOutputBuffer += 4;
outputBytesRemaining -= 4;
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ namespace System
[StackTraceHidden]
internal static class ThrowHelper
{
[DoesNotReturn]
internal static void ThrowUnreachableException()
{
throw new UnreachableException();
}

[DoesNotReturn]
internal static void ThrowArithmeticException(string message)
{
Expand Down
Loading