Skip to content

Commit 0c9fb53

Browse files
SwapnilGaikwadjtschuster
SwapnilGaikwad
authored and
jtschuster
committed
JIT ARM64-SVE: Avoid combining conditional select for reduction intrinsics (dotnet#107207)
1 parent bf6cc33 commit 0c9fb53

File tree

11 files changed

+383
-12
lines changed

11 files changed

+383
-12
lines changed

‎src/coreclr/jit/hwintrinsic.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,9 @@ enum HWIntrinsicFlag : unsigned int
229229
// (HW_Flag_BaseTypeFrom{First, Second}Arg must also be set to denote the position of the ValueTuple)
230230
HW_Flag_BaseTypeFromValueTupleArg = 0x1000000,
231231

232+
// The intrinsic is a reduce operation.
233+
HW_Flag_ReduceOperation = 0x2000000,
234+
232235
#else
233236
#error Unsupported platform
234237
#endif
@@ -998,6 +1001,12 @@ struct HWIntrinsicInfo
9981001
return (flags & HW_Flag_BaseTypeFromValueTupleArg) != 0;
9991002
}
10001003

1004+
static bool IsReduceOperation(NamedIntrinsic id)
1005+
{
1006+
const HWIntrinsicFlag flags = lookupFlags(id);
1007+
return (flags & HW_Flag_ReduceOperation) != 0;
1008+
}
1009+
10011010
static NamedIntrinsic GetScalarInputVariant(NamedIntrinsic id)
10021011
{
10031012
assert(HasScalarInputVariant(id));

‎src/coreclr/jit/hwintrinsiclistarm64sve.h‎

Lines changed: 9 additions & 9 deletions
Large diffs are not rendered by default.

‎src/coreclr/jit/lowerarmarch.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4055,9 +4055,10 @@ GenTree* Lowering::LowerHWIntrinsicCndSel(GenTreeHWIntrinsic* cndSelNode)
40554055
NamedIntrinsic nestedOp2Id = nestedOp2->AsHWIntrinsic()->GetHWIntrinsicId();
40564056

40574057
// If the nested op uses Pg/Z, then inactive lanes will result in zeros, so can only transform if
4058-
// op3 is all zeros.
4058+
// op3 is all zeros. Such a Csel operation is absorbed into the instruction when emitted. Skip this optimisation
4059+
// when the nestedOp is a reduce operation.
40594060

4060-
if (nestedOp1->IsMaskAllBitsSet() &&
4061+
if (nestedOp1->IsMaskAllBitsSet() && !HWIntrinsicInfo::IsReduceOperation(nestedOp2Id) &&
40614062
(!HWIntrinsicInfo::IsZeroingMaskedOperation(nestedOp2Id) || op3->IsVectorZero()))
40624063
{
40634064
GenTree* nestedOp2 = nestedCndSel->Op(2);

‎src/tests/Common/GenerateHWIntrinsicTests/GenerateHWIntrinsicTests_Arm.cs‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,67 @@
220220
}
221221
}";
222222

223+
const string VecReduceUnOpTest_VectorValidationLogicForCndSel = @"
224+
{
225+
var hasFailed = (mask[0] != 0 ? {ValidateReduceOpResult}: (falseVal[0] != result[0]));
226+
227+
if (hasFailed)
228+
{
229+
succeeded = false;
230+
}
231+
else
232+
{
233+
for (var i = 1; i < RetElementCount; i++)
234+
{
235+
var iterResult = (mask[i] != 0) ? 0 : falseVal[i];
236+
if (mask[i] != 0)
237+
{
238+
// Pick the trueValue
239+
if (iterResult != result[i])
240+
{
241+
succeeded = false;
242+
break;
243+
}
244+
}
245+
else
246+
{
247+
// For false, the values are merged with destination, and we do not know
248+
// those contents would be, so skip verification for them.
249+
}
250+
}
251+
}
252+
}";
253+
254+
const string VecReduceUnOpTest_VectorValidationLogicForCndSel_FalseValue = @"
255+
{
256+
var hasFailed = (mask[0] != 0) ? (trueVal[0] != result[0]): {ValidateReduceOpResult};
257+
if (hasFailed)
258+
{
259+
succeeded = false;
260+
}
261+
else
262+
{
263+
for (var i = 1; i < RetElementCount; i++)
264+
{
265+
var iterResult = (mask[i] != 0) ? trueVal[i] : 0;
266+
if (mask[i] != 0)
267+
{
268+
// Pick the trueValue
269+
if (iterResult != result[i])
270+
{
271+
succeeded = false;
272+
break;
273+
}
274+
}
275+
else
276+
{
277+
// For false, the values are merged with destination, and we do not know
278+
// those contents would be, so skip verification for them.
279+
}
280+
}
281+
}
282+
}";
283+
223284
const string VecReduceOpTest_ValidationLogic = @"if ({ValidateReduceOpResult})
224285
{
225286
succeeded = false;
@@ -293,7 +354,7 @@
293354
("_SveImmTernOpFirstArgTestTemplate.template", "SveVecImmTernOpFirstArgTest.template", new Dictionary<string, string> { ["TemplateName"] = "Simple", ["TemplateValidationLogic"] = SimpleVecOpTest_ValidationLogic, ["TemplateValidationLogicForCndSel"] = SimpleTernVecOpTest_ValidationLogicForCndSel, ["TemplateValidationLogicForCndSel_FalseValue"] = SimpleTernVecOpTest_ValidationLogicForCndSel_FalseValue }),
294355
("_SveScalarTernOpTestTemplate.template", "SveScalarTernOpTest.template", new Dictionary<string, string> { ["TemplateName"] = "Simple", ["TemplateValidationLogic"] = SimpleScalarOpTest_ValidationLogic }),
295356
("_SveImm2UnaryOpTestTemplate.template", "SveVecImm2UnOpTest.template", new Dictionary<string, string> { ["TemplateName"] = "Imm", ["TemplateValidationLogic"] = SimpleVecOpTest_ValidationLogic }),
296-
("_SveMinimalUnaryOpTestTemplate.template", "SveVecReduceUnOpTest.template", new Dictionary<string, string> { ["TemplateName"] = "Simple", ["TemplateValidationLogic"] = VecReduceOpTest_ValidationLogic }),
357+
("_SveMinimalUnaryOpTestTemplate.template", "SveVecReduceUnOpTest.template", new Dictionary<string, string> { ["TemplateName"] = "Simple", ["TemplateValidationLogic"] = VecReduceOpTest_ValidationLogic, ["TemplateValidationLogicForCndSel"] = VecReduceUnOpTest_VectorValidationLogicForCndSel, ["TemplateValidationLogicForCndSel_FalseValue"] = VecReduceUnOpTest_VectorValidationLogicForCndSel_FalseValue }),
297358
("_SveMasklessUnaryOpTestTemplate.template", "SveMasklessSimpleVecOpTest.template", new Dictionary<string, string> { ["TemplateName"] = "Simple", ["TemplateValidationLogic"] = SimpleVecOpTest_ValidationLogic }),
298359
("_SveVecAndScalarOpTest.template", "SveVecAndScalarOpTest.template", new Dictionary<string, string> { ["TemplateName"] = "Simple", ["TemplateValidationLogic"] = SimpleVecOpTest_VectorValidationLogic }),
299360
("_SveMasklessBinaryOpTestTemplate.template", "SveMasklessVecBinOpTest.template", new Dictionary<string, string> { ["TemplateName"] = "Simple", ["TemplateValidationLogic"] = SimpleVecOpTest_ValidationLogic }),

‎src/tests/JIT/HardwareIntrinsics/Arm/Shared/_SveMinimalUnaryOpTestTemplate.template‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ namespace JIT.HardwareIntrinsics.Arm
5050

5151
// Validates passing an instance member of a struct works
5252
test.RunStructFldScenario();
53+
54+
// Validates executing the test inside conditional, with op3 as falseValue
55+
test.ConditionalSelect_FalseOp();
56+
57+
// Validates executing the test inside conditional, with op3 as zero
58+
test.ConditionalSelect_ZeroOp();
5359
}
5460
else
5561
{
@@ -139,18 +145,25 @@ namespace JIT.HardwareIntrinsics.Arm
139145
private static readonly int Op1ElementCount = Unsafe.SizeOf<{Op1VectorType}<{Op1BaseType}>>() / sizeof({Op1BaseType});
140146
private static readonly int RetElementCount = Unsafe.SizeOf<{RetVectorType}<{RetBaseType}>>() / sizeof({RetBaseType});
141147

148+
private static {RetBaseType}[] _maskData = new {RetBaseType}[RetElementCount];
142149
private static {Op1BaseType}[] _data1 = new {Op1BaseType}[Op1ElementCount];
143150

151+
private {RetVectorType}<{RetBaseType}> _mask;
144152
private {Op1VectorType}<{Op1BaseType}> _fld1;
153+
private {RetVectorType}<{RetBaseType}> _falseFld;
145154

146155
private DataTable _dataTable;
147156

148157
public {TemplateName}UnaryOpTest__{TestName}()
149158
{
150159
Succeeded = true;
151160

161+
for (var i = 0; i < RetElementCount; i++) { _maskData[i] = ({RetBaseType})(TestLibrary.Generator.Get{RetBaseType}() % 2); }
162+
Unsafe.CopyBlockUnaligned(ref Unsafe.As<{RetVectorType}<{RetBaseType}>, byte>(ref _mask), ref Unsafe.As<{RetBaseType}, byte>(ref _maskData[0]), (uint)Unsafe.SizeOf<{RetVectorType}<{RetBaseType}>>());
163+
152164
for (var i = 0; i < Op1ElementCount; i++) { _data1[i] = {NextValueOp1}; }
153165
Unsafe.CopyBlockUnaligned(ref Unsafe.As<{Op1VectorType}<{Op1BaseType}>, byte>(ref _fld1), ref Unsafe.As<{Op1BaseType}, byte>(ref _data1[0]), (uint)Unsafe.SizeOf<{Op1VectorType}<{Op1BaseType}>>());
166+
Unsafe.CopyBlockUnaligned(ref Unsafe.As<{RetVectorType}<{RetBaseType}>, byte>(ref _falseFld), ref Unsafe.As<{Op1BaseType}, byte>(ref _data1[0]), (uint)Unsafe.SizeOf<{RetVectorType}<{RetBaseType}>>());
154167

155168
for (var i = 0; i < Op1ElementCount; i++) { _data1[i] = {NextValueOp1}; }
156169
_dataTable = new DataTable(_data1, new {RetBaseType}[RetElementCount], LargestVectorSize);
@@ -239,6 +252,66 @@ namespace JIT.HardwareIntrinsics.Arm
239252
test.RunStructFldScenario(this);
240253
}
241254

255+
public void ConditionalSelect_FalseOp()
256+
{
257+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_FalseOp_mask - operation in trueValue");
258+
ConditionalSelectScenario_TrueValue(_mask, _fld1, _falseFld);
259+
260+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_FalseOp_zero - operation in trueValue");
261+
ConditionalSelectScenario_TrueValue({RetVectorType}<{RetBaseType}>.Zero, _fld1, _falseFld);
262+
263+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_FalseOp_all - operation in trueValue");
264+
ConditionalSelectScenario_TrueValue({RetVectorType}<{RetBaseType}>.AllBitsSet, _fld1, _falseFld);
265+
266+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_FalseOp_mask - operation in falseValue");
267+
ConditionalSelectScenario_FalseValue(_mask, _fld1, _falseFld);
268+
269+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_FalseOp_zero - operation in falseValue");
270+
ConditionalSelectScenario_FalseValue({RetVectorType}<{RetBaseType}>.Zero, _fld1, _falseFld);
271+
272+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_FalseOp_all - operation in falseValue");
273+
ConditionalSelectScenario_FalseValue({RetVectorType}<{RetBaseType}>.AllBitsSet, _fld1, _falseFld);
274+
}
275+
276+
public void ConditionalSelect_ZeroOp()
277+
{
278+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_ZeroOp_mask - operation in trueValue");
279+
ConditionalSelectScenario_TrueValue(_mask, _fld1, {RetVectorType}<{RetBaseType}>.Zero);
280+
281+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_ZeroOp_zero - operation in trueValue");
282+
ConditionalSelectScenario_TrueValue({RetVectorType}<{RetBaseType}>.Zero, _fld1, {RetVectorType}<{RetBaseType}>.Zero);
283+
284+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_ZeroOp_all - operation in trueValue");
285+
ConditionalSelectScenario_TrueValue({RetVectorType}<{RetBaseType}>.AllBitsSet, _fld1, {RetVectorType}<{RetBaseType}>.Zero);
286+
287+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_ZeroOp_mask - operation in falseValue");
288+
ConditionalSelectScenario_FalseValue(_mask, _fld1, {RetVectorType}<{RetBaseType}>.Zero);
289+
290+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_ZeroOp_zero - operation in falseValue");
291+
ConditionalSelectScenario_FalseValue({RetVectorType}<{RetBaseType}>.Zero, _fld1, {RetVectorType}<{RetBaseType}>.Zero);
292+
293+
TestLibrary.TestFramework.BeginScenario("ConditionalSelect_ZeroOp_all - operation in falseValue");
294+
ConditionalSelectScenario_FalseValue({RetVectorType}<{RetBaseType}>.AllBitsSet, _fld1, {RetVectorType}<{RetBaseType}>.Zero);
295+
}
296+
297+
[method: MethodImpl(MethodImplOptions.AggressiveInlining)]
298+
private void ConditionalSelectScenario_TrueValue({RetVectorType}<{RetBaseType}> mask, {Op1VectorType}<{Op1BaseType}> op1, {RetVectorType}<{RetBaseType}> falseOp)
299+
{
300+
var result = Sve.ConditionalSelect(mask, {Isa}.{Method}(op1), falseOp);
301+
302+
Unsafe.Write(_dataTable.outArrayPtr, result);
303+
ValidateConditionalSelectResult_TrueValue(mask, op1, falseOp, _dataTable.outArrayPtr);
304+
}
305+
306+
[method: MethodImpl(MethodImplOptions.AggressiveInlining)]
307+
private void ConditionalSelectScenario_FalseValue({RetVectorType}<{RetBaseType}> mask, {Op1VectorType}<{Op1BaseType}> op1, {RetVectorType}<{RetBaseType}> trueOp)
308+
{
309+
var result = Sve.ConditionalSelect(mask, trueOp, {Isa}.{Method}(op1));
310+
311+
Unsafe.Write(_dataTable.outArrayPtr, result);
312+
ValidateConditionalSelectResult_FalseValue(mask, op1, trueOp, _dataTable.outArrayPtr);
313+
}
314+
242315
public void RunUnsupportedScenario()
243316
{
244317
TestLibrary.TestFramework.BeginScenario(nameof(RunUnsupportedScenario));
@@ -260,6 +333,62 @@ namespace JIT.HardwareIntrinsics.Arm
260333
}
261334
}
262335

336+
private void ValidateConditionalSelectResult_TrueValue({RetVectorType}<{RetBaseType}> maskOp, {Op1VectorType}<{Op1BaseType}> leftOp, {RetVectorType}<{RetBaseType}> falseOp, void* output, [CallerMemberName] string method = "")
337+
{
338+
{RetBaseType}[] mask = new {RetBaseType}[RetElementCount];
339+
{Op1BaseType}[] firstOp = new {Op1BaseType}[Op1ElementCount];
340+
{RetBaseType}[] falseVal = new {RetBaseType}[RetElementCount];
341+
{RetBaseType}[] result = new {RetBaseType}[RetElementCount];
342+
343+
Unsafe.WriteUnaligned(ref Unsafe.As<{RetBaseType}, byte>(ref mask[0]), maskOp);
344+
Unsafe.WriteUnaligned(ref Unsafe.As<{Op1BaseType}, byte>(ref firstOp[0]), leftOp);
345+
Unsafe.WriteUnaligned(ref Unsafe.As<{RetBaseType}, byte>(ref falseVal[0]), falseOp);
346+
Unsafe.CopyBlockUnaligned(ref Unsafe.As<{RetBaseType}, byte>(ref result[0]), ref Unsafe.AsRef<byte>(output), (uint)Unsafe.SizeOf<{RetVectorType}<{RetBaseType}>>());
347+
348+
bool succeeded = true;
349+
{TemplateValidationLogicForCndSel}
350+
351+
if (!succeeded)
352+
{
353+
TestLibrary.TestFramework.LogInformation($"{nameof(Sve)}.{nameof({Isa}.{Method})}<{RetBaseType}>({RetVectorType}<{RetBaseType}>, {RetVectorType}<{RetBaseType}>): {method} failed:");
354+
TestLibrary.TestFramework.LogInformation($" mask: ({string.Join(", ", mask)})");
355+
TestLibrary.TestFramework.LogInformation($" firstOp: ({string.Join(", ", firstOp)})");
356+
TestLibrary.TestFramework.LogInformation($" falseOp: ({string.Join(", ", falseVal)})");
357+
TestLibrary.TestFramework.LogInformation($" result: ({string.Join(", ", result)})");
358+
TestLibrary.TestFramework.LogInformation(string.Empty);
359+
360+
Succeeded = false;
361+
}
362+
}
363+
364+
private void ValidateConditionalSelectResult_FalseValue({RetVectorType}<{RetBaseType}> maskOp, {Op1VectorType}<{Op1BaseType}> leftOp, {RetVectorType}<{RetBaseType}> trueOp, void* output, [CallerMemberName] string method = "")
365+
{
366+
{RetBaseType}[] mask = new {RetBaseType}[RetElementCount];
367+
{Op1BaseType}[] firstOp = new {Op1BaseType}[Op1ElementCount];
368+
{RetBaseType}[] trueVal = new {RetBaseType}[RetElementCount];
369+
{RetBaseType}[] result = new {RetBaseType}[RetElementCount];
370+
371+
Unsafe.WriteUnaligned(ref Unsafe.As<{RetBaseType}, byte>(ref mask[0]), maskOp);
372+
Unsafe.WriteUnaligned(ref Unsafe.As<{Op1BaseType}, byte>(ref firstOp[0]), leftOp);
373+
Unsafe.WriteUnaligned(ref Unsafe.As<{RetBaseType}, byte>(ref trueVal[0]), trueOp);
374+
Unsafe.CopyBlockUnaligned(ref Unsafe.As<{RetBaseType}, byte>(ref result[0]), ref Unsafe.AsRef<byte>(output), (uint)Unsafe.SizeOf<{RetVectorType}<{RetBaseType}>>());
375+
376+
bool succeeded = true;
377+
{TemplateValidationLogicForCndSel_FalseValue}
378+
379+
if (!succeeded)
380+
{
381+
TestLibrary.TestFramework.LogInformation($"{nameof(Sve)}.{nameof({Isa}.{Method})}<{RetBaseType}>({RetVectorType}<{RetBaseType}>, {RetVectorType}<{RetBaseType}>): {method} failed:");
382+
TestLibrary.TestFramework.LogInformation($" mask: ({string.Join(", ", mask)})");
383+
TestLibrary.TestFramework.LogInformation($"firstOp: ({string.Join(", ", firstOp)})");
384+
TestLibrary.TestFramework.LogInformation($" trueOp: ({string.Join(", ", trueVal)})");
385+
TestLibrary.TestFramework.LogInformation($" result: ({string.Join(", ", result)})");
386+
TestLibrary.TestFramework.LogInformation(string.Empty);
387+
388+
Succeeded = false;
389+
}
390+
}
391+
263392
private void ValidateResult({Op1VectorType}<{Op1BaseType}> op1, void* result, [CallerMemberName] string method = "")
264393
{
265394
{Op1BaseType}[] inArray1 = new {Op1BaseType}[Op1ElementCount];

‎src/tests/JIT/Regression/JitBlue/Runtime_106868/Runtime_106868.cs‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
using System.Runtime.CompilerServices;
4+
using Xunit;
5+
6+
// Generated by Fuzzlyn v2.3 on 2024-08-23 10:17:54
7+
// Run on Arm64 Windows
8+
// Seed: 14752078066107523191-vectort,vector64,vector128,armsve
9+
// Reduced from 52.7 KiB to 0.7 KiB in 00:00:54
10+
// Hits JIT assert in Release:
11+
// Assertion failed '!"Got unexpected instruction format after MOVPRFX"' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'Emit code' (IL size 54; hash 0xade6b36b; FullOpts)
12+
//
13+
// File: C:\dev\dotnet\runtime2\src\coreclr\jit\emitarm64sve.cpp Line: 18623
14+
//
15+
16+
using System;
17+
using System.Numerics;
18+
using System.Runtime.Intrinsics;
19+
using System.Runtime.Intrinsics.Arm;
20+
21+
public struct S2
22+
{
23+
public Vector<int> F2;
24+
}
25+
26+
public class Runtime_106868
27+
{
28+
public static S2 s_1;
29+
30+
public static int M4()
31+
{
32+
Vector<long> vr17 = default(Vector<long>);
33+
var vr11 = Vector.Create<long>(0);
34+
var vr8 = Sve.SubtractSaturate(vr17, vr11);
35+
return 1;
36+
}
37+
38+
[Fact]
39+
public static void TestEntryPoint()
40+
{
41+
if (Sve.IsSupported)
42+
{
43+
var vr12 = Vector.Create<int>(0);
44+
var vr13 = Vector.Create<int>(0);
45+
var vr14 = M4();
46+
var vr15 = Vector128.CreateScalar(vr14).AsVector();
47+
var vr16 = Sve.AndAcross(vr13);
48+
s_1.F2 = Sve.ConditionalSelect(vr12, vr16, vr15);
49+
Consume(s_1.F2);
50+
}
51+
}
52+
53+
[MethodImpl(MethodImplOptions.NoInlining)]
54+
private static void Consume(Vector<int> v)
55+
{
56+
}
57+
}

‎src/tests/JIT/Regression/JitBlue/Runtime_106868/Runtime_106868.csproj‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<Optimize>True</Optimize>
4+
<NoWarn>$(NoWarn),SYSLIB5003</NoWarn>
5+
</PropertyGroup>
6+
<ItemGroup>
7+
<Compile Include="$(MSBuildProjectName).cs" />
8+
</ItemGroup>
9+
</Project>

0 commit comments

Comments
 (0)