From ba1d58a4132e7b63ee9aa19f0de8fd864405d491 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Wed, 28 Aug 2024 10:06:59 +0100 Subject: [PATCH 1/4] ARM64-SVE: Allow SVE ops to re-use the same registers --- src/coreclr/jit/hwintrinsiccodegenarm64.cpp | 7 --- .../Shared/_SveBinaryOpTestTemplate.template | 29 ++++++++++++ .../Shared/_SveTernOpTestTemplate.template | 30 ++++++++++++ .../JitBlue/Runtime_106866/Runtime_106866.cs | 47 +++++++++++++++++++ .../Runtime_106866/Runtime_106866.csproj | 9 ++++ 5 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.csproj diff --git a/src/coreclr/jit/hwintrinsiccodegenarm64.cpp b/src/coreclr/jit/hwintrinsiccodegenarm64.cpp index f5b5c1e8535455..f9bbad66dff62e 100644 --- a/src/coreclr/jit/hwintrinsiccodegenarm64.cpp +++ b/src/coreclr/jit/hwintrinsiccodegenarm64.cpp @@ -1106,7 +1106,6 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) } else if (isRMW) { - assert((targetReg == op1Reg) || (targetReg != op2Reg)); GetEmitter()->emitIns_Mov(INS_mov, emitTypeSize(node), targetReg, op1Reg, /* canSkip */ true); GetEmitter()->emitIns_R_R(ins, emitSize, targetReg, op2Reg, opt); @@ -1124,18 +1123,12 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) { if (HWIntrinsicInfo::IsExplicitMaskedOperation(intrin.id)) { - assert((targetReg == op1Reg) || (targetReg != op1Reg)); - assert((targetReg == op1Reg) || (targetReg != op3Reg)); - GetEmitter()->emitIns_Mov(INS_mov, emitTypeSize(node), targetReg, op2Reg, /* canSkip */ true); GetEmitter()->emitIns_R_R_R(ins, emitSize, targetReg, op1Reg, op3Reg, opt); } else { - assert((targetReg == op1Reg) || (targetReg != op2Reg)); - assert((targetReg == op1Reg) || (targetReg != op3Reg)); - GetEmitter()->emitIns_Mov(INS_mov, emitTypeSize(node), targetReg, op1Reg, /* canSkip */ true); GetEmitter()->emitIns_R_R_R(ins, emitSize, targetReg, op2Reg, op3Reg, opt); diff --git a/src/tests/JIT/HardwareIntrinsics/Arm/Shared/_SveBinaryOpTestTemplate.template b/src/tests/JIT/HardwareIntrinsics/Arm/Shared/_SveBinaryOpTestTemplate.template index 3ffe8ca273b954..06133b661e58d3 100644 --- a/src/tests/JIT/HardwareIntrinsics/Arm/Shared/_SveBinaryOpTestTemplate.template +++ b/src/tests/JIT/HardwareIntrinsics/Arm/Shared/_SveBinaryOpTestTemplate.template @@ -43,9 +43,15 @@ namespace JIT.HardwareIntrinsics.Arm // Validates passing a local works, using Unsafe.Read test.RunLclVarScenario_UnsafeRead(); + // Validates using the same local multiple times works, using Unsafe.Read + test.RunSameLclVarScenario_UnsafeRead(); + // Validates passing an instance member of a class works test.RunClassFldScenario(); + // Validates using the same instance member of a class multiple times works + test.RunSameClassFldScenario(); + // Validates passing the field of a local struct works test.RunStructLclFldScenario(); @@ -250,6 +256,19 @@ namespace JIT.HardwareIntrinsics.Arm ValidateResult(op1, op2, _dataTable.outArrayPtr); } + public void RunSameLclVarScenario_UnsafeRead() + { + TestLibrary.TestFramework.BeginScenario(nameof(RunSameLclVarScenario_UnsafeRead)); + + var op = Unsafe.Read<{Op1VectorType}<{Op1BaseType}>>(_dataTable.inArray1Ptr); + var op1 = op; + var op2 = op; + var result = {Isa}.{Method}(op1, op2); + + Unsafe.Write(_dataTable.outArrayPtr, result); + ValidateResult(op1, op2, _dataTable.outArrayPtr); + } + public void RunClassFldScenario() { TestLibrary.TestFramework.BeginScenario(nameof(RunClassFldScenario)); @@ -260,6 +279,16 @@ namespace JIT.HardwareIntrinsics.Arm ValidateResult(_fld1, _fld2, _dataTable.outArrayPtr); } + public void RunSameClassFldScenario() + { + TestLibrary.TestFramework.BeginScenario(nameof(RunSameClassFldScenario)); + + var result = {Isa}.{Method}(_fld1, _fld1); + + Unsafe.Write(_dataTable.outArrayPtr, result); + ValidateResult(_fld1, _fld1, _dataTable.outArrayPtr); + } + public void RunStructLclFldScenario() { TestLibrary.TestFramework.BeginScenario(nameof(RunStructLclFldScenario)); diff --git a/src/tests/JIT/HardwareIntrinsics/Arm/Shared/_SveTernOpTestTemplate.template b/src/tests/JIT/HardwareIntrinsics/Arm/Shared/_SveTernOpTestTemplate.template index 4c3334cb1475c8..13343926763ced 100644 --- a/src/tests/JIT/HardwareIntrinsics/Arm/Shared/_SveTernOpTestTemplate.template +++ b/src/tests/JIT/HardwareIntrinsics/Arm/Shared/_SveTernOpTestTemplate.template @@ -43,9 +43,15 @@ namespace JIT.HardwareIntrinsics.Arm // Validates passing a local works, using Unsafe.Read test.RunLclVarScenario_UnsafeRead(); + // Validates using the same local multiple times works, using Unsafe.Read + test.RunSameLclVarScenario_UnsafeRead(); + // Validates passing an instance member of a class works test.RunClassFldScenario(); + // Validates using the same instance member of a class multiple times works + test.RunSameClassFldScenario(); + // Validates passing the field of a local struct works test.RunStructLclFldScenario(); @@ -277,6 +283,20 @@ namespace JIT.HardwareIntrinsics.Arm ValidateResult(op1, op2, op3, _dataTable.outArrayPtr); } + public void RunSameLclVarScenario_UnsafeRead() + { + TestLibrary.TestFramework.BeginScenario(nameof(RunSameLclVarScenario_UnsafeRead)); + + var op = Unsafe.Read<{Op1VectorType}<{Op1BaseType}>>(_dataTable.inArray1Ptr); + var op1 = op; + var op2 = op; + var op3 = op; + var result = {Isa}.{Method}(op1, op2, op3); + + Unsafe.Write(_dataTable.outArrayPtr, result); + ValidateResult(op1, op2, op3, _dataTable.outArrayPtr); + } + public void RunClassFldScenario() { TestLibrary.TestFramework.BeginScenario(nameof(RunClassFldScenario)); @@ -287,6 +307,16 @@ namespace JIT.HardwareIntrinsics.Arm ValidateResult(_fld1, _fld2, _fld3, _dataTable.outArrayPtr); } + public void RunSameClassFldScenario() + { + TestLibrary.TestFramework.BeginScenario(nameof(RunSameClassFldScenario)); + + var result = {Isa}.{Method}(_fld1, _fld1, _fld1); + + Unsafe.Write(_dataTable.outArrayPtr, result); + ValidateResult(_fld1, _fld1, _fld1, _dataTable.outArrayPtr); + } + public void RunStructLclFldScenario() { TestLibrary.TestFramework.BeginScenario(nameof(RunStructLclFldScenario)); diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.cs b/src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.cs new file mode 100644 index 00000000000000..977d417bb75514 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.cs @@ -0,0 +1,47 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Xunit; +using System.Runtime.CompilerServices; + +// Generated by Fuzzlyn v2.3 on 2024-08-23 10:04:52 +// Run on Arm64 Windows +// Seed: 12028719405363964033-vectort,vector64,vector128,armsve +// Reduced from 60.4 KiB to 0.7 KiB in 00:00:33 +// Hits JIT assert in Release: +// Assertion failed '(targetReg == op1Reg) || (targetReg != op3Reg)' in 'S0:M3():this' during 'Generate code' (IL size 57; hash 0x4541fc9f; FullOpts) +// +// File: C:\dev\dotnet\runtime2\src\coreclr\jit\hwintrinsiccodegenarm64.cpp Line: 1128 +// +using System; +using System.Numerics; +using System.Runtime.Intrinsics; +using System.Runtime.Intrinsics.Arm; + +public struct S0 +{ + public bool F0; + public Vector F2; + public void M3() + { + var vr0 = this.F2; + var vr1 = this.F2; + var vr2 = this.F2; + this.F2 = Sve.Splice(vr0, vr1, vr2); + Consume(this.F0); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void Consume(T val) + { + } +} + +public class Runtime_106866 +{ + [Fact] + public static void TestEntryPoint() + { + new S0().M3(); + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.csproj new file mode 100644 index 00000000000000..1352ebe3277bc7 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.csproj @@ -0,0 +1,9 @@ + + + True + $(NoWarn),SYSLIB5003 + + + + + From 6b64ef8a7ae46eaccdc289509d6026486721fca9 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Mon, 9 Sep 2024 11:57:01 +0100 Subject: [PATCH 2/4] Add Sve.IsSupported --- .../JitBlue/Runtime_106866/Runtime_106866.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.cs b/src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.cs index 977d417bb75514..62e2058917cdac 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.cs @@ -24,11 +24,14 @@ public struct S0 public Vector F2; public void M3() { - var vr0 = this.F2; - var vr1 = this.F2; - var vr2 = this.F2; - this.F2 = Sve.Splice(vr0, vr1, vr2); - Consume(this.F0); + if (Sve.IsSupported) + { + var vr0 = this.F2; + var vr1 = this.F2; + var vr2 = this.F2; + this.F2 = Sve.Splice(vr0, vr1, vr2); + Consume(this.F0); + } } [MethodImpl(MethodImplOptions.NoInlining)] From 554d1390e4b2a79fe7fd96cedbcf1298e55e8f00 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Fri, 13 Sep 2024 09:43:27 +0100 Subject: [PATCH 3/4] restore an assert --- src/coreclr/jit/hwintrinsiccodegenarm64.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/hwintrinsiccodegenarm64.cpp b/src/coreclr/jit/hwintrinsiccodegenarm64.cpp index 92590b613bb17e..6ca4717a088fd5 100644 --- a/src/coreclr/jit/hwintrinsiccodegenarm64.cpp +++ b/src/coreclr/jit/hwintrinsiccodegenarm64.cpp @@ -1096,6 +1096,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) } else if (isRMW) { + assert((targetReg == op1Reg) || (targetReg != op2Reg)); GetEmitter()->emitIns_Mov(INS_mov, emitTypeSize(node), targetReg, op1Reg, /* canSkip */ true); GetEmitter()->emitIns_R_R(ins, emitSize, targetReg, op2Reg, opt); From b8b0fb53b3b5832e0ecff32039ce0ecf06c34bec Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Fri, 13 Sep 2024 18:38:05 +0100 Subject: [PATCH 4/4] better asserts --- src/coreclr/jit/hwintrinsiccodegenarm64.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/jit/hwintrinsiccodegenarm64.cpp b/src/coreclr/jit/hwintrinsiccodegenarm64.cpp index 6ca4717a088fd5..6e447757c8ce19 100644 --- a/src/coreclr/jit/hwintrinsiccodegenarm64.cpp +++ b/src/coreclr/jit/hwintrinsiccodegenarm64.cpp @@ -1114,12 +1114,16 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) { if (HWIntrinsicInfo::IsExplicitMaskedOperation(intrin.id)) { + assert((targetReg == op2Reg) || ((targetReg != op1Reg) && (targetReg != op3Reg))); + GetEmitter()->emitIns_Mov(INS_mov, emitTypeSize(node), targetReg, op2Reg, /* canSkip */ true); GetEmitter()->emitIns_R_R_R(ins, emitSize, targetReg, op1Reg, op3Reg, opt); } else { + assert((targetReg == op1Reg) || ((targetReg != op2Reg) && (targetReg != op3Reg))); + GetEmitter()->emitIns_Mov(INS_mov, emitTypeSize(node), targetReg, op1Reg, /* canSkip */ true); GetEmitter()->emitIns_R_R_R(ins, emitSize, targetReg, op2Reg, op3Reg, opt);