Fix matrix multiplication (#141)

* fix matrix multiplication bug * fix stackoverflow * rename test method * fix unit test for mac * add comment * improve can multiply test
toor1245 · Jun 23, 2021 · 151ac23 · 151ac23
1 parent f819c9d
commit 151ac23
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 8 deletions.
diff --git a/src/MatrixDotNet/Extensions/Performance/Simd/Handler/IntrinsicsHandler.cs b/src/MatrixDotNet/Extensions/Performance/Simd/Handler/IntrinsicsHandler.cs
@@ -11,6 +11,25 @@ internal static class IntrinsicsHandler<T> where T : unmanaged
     {
         #region Vector256
 
+        private static Vector256<int> _blockMultiplyMask = Vector256.Create(16, 32, 64, 128, 256, 512, 1024, 2048);
+
+        /// <summary>
+        /// Determines whether a sequence contains a specified value through SIMD.
+        /// </summary>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal static bool ContainsMultiplyMask(int value)
+        {
+            var vector = Vector256.Create(value);
+            var mask = Avx2.CompareEqual(vector, _blockMultiplyMask);
+            return !Avx.TestZ(mask, mask);
+        }
+
+        /// <summary>
+        /// Determines whether a specified value can multiply through SIMD.
+        /// </summary>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal static bool CanMultiply(int value) => Avx2.IsSupported && IsSupportedMultiplyAddVector256 && ContainsMultiplyMask(value);
+
         /// <summary>
         /// Gets a new Vector256<T/> with all bits set to 1.
         /// </summary>

diff --git a/src/MatrixDotNet/Matrix.cs b/src/MatrixDotNet/Matrix.cs
@@ -102,12 +102,6 @@ public bool IsSymmetric
             }
         }
 
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        internal bool CanMultiplyBlock(Matrix<T> right)
-        {
-            return Columns == right.Rows && Columns >= 0b1000 && ((Columns & 0b100) == 0 || Columns == 0b1000);
-        }
-
         #endregion
 
         #region indexators
@@ -467,7 +461,7 @@ public unsafe Matrix(int row, int col, T value)
                 var span1 = new Span<T>(ptrA, length);
 
 #if NET5_0 || NETCOREAPP3_1
-                if (Fma.IsSupported && IntrinsicsHandler<T>.IsSupportedMultiplyAddVector256 && left.CanMultiplyBlock(right))
+                if (IntrinsicsHandler<T>.CanMultiply(m))
                 {
                     var size = Vector256<T>.Count;
                     for (int i = 0; i < m; i++)

diff --git a/tests/MatrixDotNet.UnitTesting/MatrixTests/BasicOperations.cs b/tests/MatrixDotNet.UnitTesting/MatrixTests/BasicOperations.cs
@@ -1,4 +1,6 @@
 using System;
+using System.Linq;
+using System.Runtime.CompilerServices;
 using System.Threading.Tasks;
 using MatrixDotNet;
 using MatrixDotNet.Exceptions;
@@ -87,7 +89,26 @@ public void MatrixMultiplySimdDoubleTest_MultiplyTwoMatrix_AssertMustBeEqual()
             // Assert
             Assert.Equal(expected, actual);
         }
-
+
+        [Theory]
+        [InlineData(8)]
+        [InlineData(16)]
+        [InlineData(17)]
+        [InlineData(21)]
+        [InlineData(32)]
+        public void MatrixMultiply_CanMultiply(int size)
+        {
+            // Arrange
+            var matrix = new Matrix<double>(size, size, 1);
+            var expected = Enumerable.Repeat((double) size, size * size);
+
+            // Act
+            var actual = matrix * matrix;
+
+            // Assert
+            Assert.Equal(expected, actual);
+        }
+
         [Theory]
         [InlineData(16)]
         [InlineData(32)]