giladfrid009/simplesimd

Easy to use SIMD accelerated Array and Span methods

simplesimd's Issues

MathOps operations do not cast correctly.

This results in an InvalidCastException:

Console.WriteLine(MathOps<byte>.Add(1, 1));

The reason is that C# does not define operators for smaller types. So code like this is wrong as it attempts to unbox an Int32 to Byte:

return (T)(object)((byte)(object)left / (byte)(object)right);

The correct version would be the following:

return (T)(object)(byte)((byte)(object)left / (byte)(object)right);

This needs to be fixed for byte, sbyte, short and ushort.

Swapo?

SimpleSIMD/SimpleSIMD/Elementwise/Divide.cs

Line 57 in 5e486d9

var vLeft = new Vector<T>(right);

Did you swap left for right there?

Consider using throw helpers to improve code quality.

In high performance C# code, it is common to use "throw helpers" - methods that simply throw. The reason for doing so is smaller machine code size (less memory used, less pressure on CPU caches, but most significantly - better chance of inlining). For example:

public static int AddWithThrow(int i, int j)
{
    if (i < 0)
        throw new ArgumentOutOfRangeException(nameof(i));

    return i + j;
}

public static int AddWithThrowHelper(int i, int j)
{
    if (i < 0)
        ThrowArgumentOutOfRangeException(nameof(i));

    return i + j;
}

public static int ThrowArgumentOutOfRangeException(string argName) => throw new ArgumentOutOfRangeException(argName);

C.AddWithThrow(Int32, Int32)
    L0000: push esi
    L0001: test ecx, ecx
    L0003: jl short L000a
    L0005: lea eax, [ecx+edx]
    L0008: pop esi
    L0009: ret
    L000a: mov ecx, 0x3912fb8
    L000f: call 0x02fb30cc
    L0014: mov esi, eax
    L0016: mov ecx, 1
    L001b: mov edx, 0x1191c040
    L0020: call 0x5e76b3d0
    L0025: mov edx, eax
    L0027: mov ecx, esi
    L0029: call System.ArgumentOutOfRangeException..ctor(System.String)
    L002e: mov ecx, esi
    L0030: call 0x5e750cb0
    L0035: int3

C.AddWithThrowHelper(Int32, Int32)
    L0000: push edi
    L0001: push esi
    L0002: mov esi, ecx
    L0004: mov edi, edx
    L0006: test esi, esi
    L0008: jge short L0016
    L000a: mov ecx, [0xe5168c4]
    L0010: call dword ptr [0x1191c6f0]
    L0016: lea eax, [esi+edi]
    L0019: pop esi
    L001a: pop edi
    L001b: ret

Note that our "happy" path actually significantly regressed - this is because the JIT thinks that our branch is actually "happy" and optimizes for that (while it automatically detects all branches with throw as "unhappy" and reorders the code accordingly). We can improve the codegen if we hint to the JIT that out "throw" path is an "unhappy" one:

public static int AddWithThrowHelper(int i, int j)
{
    if (i >= 0)
        return i + j;

    return ThrowArgumentOutOfRangeException(nameof(i));
}

C.AddWithThrowHelper(Int32, Int32)
    L0000: test ecx, ecx
    L0002: jle short L0008
    L0004: lea eax, [ecx+edx]
    L0007: ret
    L0008: mov ecx, [0xe5168c4]
    L000e: call dword ptr [0x13f6c6f0]
    L0014: ret

Code size improvements like these matter most when talking about small methods, like the one above. Of all its versions, likely only the last one would get considered for inlining.

Throw helpers have a cost in terms of maintainability and readability, mainly due to the fact the order of branches can now change the generated code so significantly. Thus, as always, the benefits have to weighed against the costs, and benchmarks run with motivating scenarios in mind.

Swapo #2

SimpleSIMD/SimpleSIMD/Elementwise/Subtract.cs

Line 57 in 5e486d9

var vLeft = new Vector<T>(right);

Same here?