From 0ac6b9970e149014fb11890c865c12ae6cdd747b Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Fri, 13 Sep 2024 21:52:11 +0100 Subject: [PATCH 1/2] ARM64-SVE: Allow SVE ops to re-use the same registers (#107084) * ARM64-SVE: Allow SVE ops to re-use the same registers * Add Sve.IsSupported * restore an assert * better asserts --- src/coreclr/jit/hwintrinsiccodegenarm64.cpp | 11 +--- .../Shared/_SveBinaryOpTestTemplate.template | 29 +++++++++++ .../Shared/_SveTernOpTestTemplate.template | 30 +++++++++++ .../JitBlue/Runtime_106866/Runtime_106866.cs | 50 +++++++++++++++++++ .../Runtime_106866/Runtime_106866.csproj | 9 ++++ 5 files changed, 120 insertions(+), 9 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 32492363d548ad..f7e7f5b5f8351b 100644 --- a/src/coreclr/jit/hwintrinsiccodegenarm64.cpp +++ b/src/coreclr/jit/hwintrinsiccodegenarm64.cpp @@ -1141,10 +1141,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) { if (HWIntrinsicInfo::IsExplicitMaskedOperation(intrin.id)) { - if (targetReg != op2Reg) - { - assert(targetReg != op1Reg); - assert(targetReg != op3Reg); + assert((targetReg == op2Reg) || ((targetReg != op1Reg) && (targetReg != op3Reg))); GetEmitter()->emitIns_Mov(INS_mov, emitTypeSize(node), targetReg, op2Reg, /* canSkip */ true); @@ -1154,11 +1151,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) } else { - if (targetReg != op1Reg) - { - assert(targetReg != op2Reg); - assert(targetReg != op3Reg); - + assert((targetReg == op1Reg) || ((targetReg != op2Reg) && (targetReg != op3Reg))); GetEmitter()->emitIns_Mov(INS_mov, emitTypeSize(node), targetReg, op1Reg, /* canSkip */ true); } 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..62e2058917cdac --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_106866/Runtime_106866.cs @@ -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 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() + { + 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)] + 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 fd0aaa64e084434d6a021c13bf64b76deb66d31d Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Fri, 13 Sep 2024 16:28:04 -0700 Subject: [PATCH 2/2] resolve merge conflicts --- src/coreclr/jit/hwintrinsiccodegenarm64.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/hwintrinsiccodegenarm64.cpp b/src/coreclr/jit/hwintrinsiccodegenarm64.cpp index f7e7f5b5f8351b..ff64d23f6f58b9 100644 --- a/src/coreclr/jit/hwintrinsiccodegenarm64.cpp +++ b/src/coreclr/jit/hwintrinsiccodegenarm64.cpp @@ -1141,20 +1141,20 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) { if (HWIntrinsicInfo::IsExplicitMaskedOperation(intrin.id)) { - assert((targetReg == op2Reg) || ((targetReg != op1Reg) && (targetReg != op3Reg))); + assert((targetReg == op2Reg) || ((targetReg != op1Reg) && (targetReg != op3Reg))); - GetEmitter()->emitIns_Mov(INS_mov, emitTypeSize(node), targetReg, op2Reg, - /* canSkip */ true); - } + 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); - } + 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); } }