[Arm64] Support for Vector Intrinsics and/or SIMD requires 128bit argument registers to be preserved

[sdmaclea]

@dotnet/jit-contrib @dotnet/arm64-contrib

The following logic and assumptions will no longer be true when SIMD intrinsic support is added.

//--------------------------------------------------------------------
// This represents the floating point argument registers which are saved
// as part of the NegInfo for a FramedMethodFrame. Note that these
// might not be saved by all stubs: typically only those that call into
// C++ helpers will need to preserve the values in these volatile
// registers.
//--------------------------------------------------------------------
typedef DPTR(struct FloatArgumentRegisters) PTR_FloatArgumentRegisters;
struct FloatArgumentRegisters {
    // armV8 supports 32 floating point registers. Each register is 128bits long.
    // It can be accessed as 128-bit value or 64-bit value(d0-d31) or as 32-bit value (s0-s31)
    // or as 16-bit value or as 8-bit values. C# only has two builtin floating datatypes float(32-bit) and 
    // double(64-bit). It does not have a quad-precision floating point.So therefore it does not make sense to
    // store full 128-bit values in Frame when the upper 64 bit will not contain any values.
    double  d[8];  // d0-d7
};

I plan to change this to save ful 128-bits all the time. If there are any objections, let me know.

This will also affect SAVE_FLOAT_ARGUMENT_REGISTERS and RESTORE_FLOAT_ARGUMENT_REGISTERS macros and related assembly frame layouts

category:correctness
theme:vector-codegen
skill-level:expert
cost:small

[jkotas]

Are you planning to change the ABI on ARM64? We try to stick with the default platform calling convention.

[sdmaclea]

C++ ARM64 ABI is not being followed by CoreCLR in this case. We are only preserving the lower 64 bits of the 128-bit floating point argument registers.

This is based on an "optimization" noted above that since CoreCLR only uses double in operands there is no reason to preserve the upper 64 bits. I can look at the ABI again, but I do not see anything prohibiting passing 128-bit arguments in floating point registers.

I haven't actually found a test that hits this issue. When I do I will have motivation to change the code to conform to the ARM64 ABI.

[jkotas]

The 128-bit arguments passed in floating point registers are HVAs. CoreCLR does not support HVAs today - there is no way to say "this type is HVA".

[sdmaclea]

The 128-bit arguments passed in floating point registers are HVAs. CoreCLR does not support HVAs today - there is no way to say "this type is HVA".

I am not a compiler guy, so you'll have to explain the HVA acronym.

I was suspecting SIMD/Intrinsics would add this behavior.

Vector128.cs adds

namespace System.Runtime.Intrinsics
{
    [StructLayout(LayoutKind.Sequential, Size = 16)]
    public struct Vector128<T> where T : struct {}
}

The Intel intrinsics make it look like they are being passed as arguments to the intrinsic.

Intel does support passing structs in registers FEATURE_UNIX_AMD64_STRUCT_PASSING. I think the ARM64 ABI is supposed to have the same support, but it is not enabled in CoreCLR at the moment.

Also Intel is preserving the whole YMM 128-bit register. Presumably for the same reason.

[sdmaclea]

OK. Found the acronym. https://blogs.msdn.microsoft.com/vcblog/2013/07/11/introducing-vector-calling-convention/

[jkotas]

The hardware intrinsics are inlined most of the time. In the rare cases where the hardware intrinsics are called as method (e.g. if you call them via reflection), the arguments are passed to them as regular arguments (not HVAs) today.

[sdmaclea]

Sound like this should be closed until HVAs are on the roadmap

[tannergooding]

Related, but I have a proposal to support the __vectorcall calling convention here: https://github.com/dotnet/coreclr/issues/12120

[sdmaclea]

Reopening because Vector64<T> & Vector128<T> will be treated as Short Vectors and therefore Structs containing them will be able to be treated as HVA