Skip to content

Commit

Permalink
Ensure we consistently broadcast the result of simd dot product (#106031
Browse files Browse the repository at this point in the history
)
  • Loading branch information
tannergooding authored Aug 14, 2024
1 parent 53780c9 commit 42ababd
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 15 deletions.
9 changes: 4 additions & 5 deletions src/coreclr/jit/lowerxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4865,8 +4865,9 @@ GenTree* Lowering::LowerHWIntrinsicDot(GenTreeHWIntrinsic* node)

horizontalAdd = NI_SSE3_HorizontalAdd;

if (!comp->compOpportunisticallyDependsOn(InstructionSet_SSE3))
if ((simdSize == 8) || !comp->compOpportunisticallyDependsOn(InstructionSet_SSE3))
{
// We also do this for simdSize == 8 to ensure we broadcast the result as expected
shuffle = NI_SSE_Shuffle;
}
break;
Expand Down Expand Up @@ -4917,10 +4918,8 @@ GenTree* Lowering::LowerHWIntrinsicDot(GenTreeHWIntrinsic* node)

horizontalAdd = NI_SSE3_HorizontalAdd;

if (!comp->compOpportunisticallyDependsOn(InstructionSet_SSE3))
{
shuffle = NI_SSE2_Shuffle;
}
// We need to ensure we broadcast the result as expected
shuffle = NI_SSE2_Shuffle;
break;
}

Expand Down
10 changes: 0 additions & 10 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10724,16 +10724,6 @@ GenTree* Compiler::fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node)
break;
}

#if defined(TARGET_XARCH)
if ((node->GetSimdSize() == 8) && !compOpportunisticallyDependsOn(InstructionSet_SSE41))
{
// When SSE4.1 isn't supported then Vector2 only needs a single horizontal add
// which means the result isn't broadcast across the entire vector and we can't
// optimize
break;
}
#endif // TARGET_XARCH

GenTree* op1 = node->Op(1);
GenTree* sqrt = nullptr;
GenTree* toScalar = nullptr;
Expand Down
50 changes: 50 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_99391/Runtime_99391.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.CompilerServices;
using System.Numerics;
using Xunit;

public class Runtime_99391
{
[Fact]
public static void TestEntryPoint()
{
Vector2 result2a = Vector2.Normalize(Value2);
Assert.Equal(new Vector2(0, 1), result2a);

Vector2 result2b = Vector2.Normalize(new Vector2(0, 2));
Assert.Equal(new Vector2(0, 1), result2b);

Vector3 result3a = Vector3.Normalize(Value3);
Assert.Equal(new Vector3(0, 0, 1), result3a);

Vector3 result3b = Vector3.Normalize(new Vector3(0, 0, 2));
Assert.Equal(new Vector3(0, 0, 1), result3b);

Vector4 result4a = Vector4.Normalize(Value4);
Assert.Equal(new Vector4(0, 0, 0, 1), result4a);

Vector4 result4b = Vector4.Normalize(new Vector4(0, 0, 0, 2));
Assert.Equal(new Vector4(0, 0, 0, 1), result4b);
}

private static Vector2 Value2
{
[MethodImpl(MethodImplOptions.NoInlining)]
get => new Vector2(0, 2);
}

private static Vector3 Value3
{
[MethodImpl(MethodImplOptions.NoInlining)]
get => new Vector3(0, 0, 2);
}

private static Vector4 Value4
{
[MethodImpl(MethodImplOptions.NoInlining)]
get => new Vector4(0, 0, 0, 2);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
<ItemGroup>
<CLRTestEnvironmentVariable Include="DOTNET_TieredCompilation" Value="0" />
</ItemGroup>
</Project>

0 comments on commit 42ababd

Please sign in to comment.