Removing the legacy helper intrinsics and adding tests for their replacements

[tannergooding]

CC. @CarolEidt, @fiigii

All the new tests are templated and should also be running on ARM (given the software fallback).
I will address the JIT hookups in a follow-up PR, given the size of the PR already.

[fiigii]

So, this PR just tests the software fallback, and you will make the JIT implementation in another PR, right?

[tannergooding]

So, this PR just tests the software fallback, and you will make the JIT implementation in another PR, right?

Exactly right.

[tannergooding]

The Create methods take args from lowest index to highest index; which is opposite what SetVector did

[tannergooding]

Same with Create vs SetHighLow, it takes lower then upper; rather than upper then lower

[tannergooding]

Output of the PacketTracer test looks correct as well:

[tannergooding]

Still TODO for the tests:

• GetLower, GetUpper, WithLower, and WithUpper
• ToScalar
• ToVector128, ToVector256, ToVector128Unsafe, and ToVector256Unsafe

[tannergooding]

@CarolEidt, @fiigii. This should provide templated test coverage for all the new helper APIs.

As with most of these HWIntrinsic test PRs, I recommend only viewing the product changes, the new templates, and the changes to the GenerateTests.csx file. The actual tests, being generated from templates, are all the same and have been contained to the final commit (and represent the bulk, 45k lines, of the changes).

[tannergooding]

Also CC. @eerhardt, who was helped reviews these in the past

[CarolEidt]

I second @fiigii's suggestion to add testing for exceptional cases, and have a question and another comment. Otherwise, LGTM, though I confess I got a little cross-eyed reading through all this ;-)

[tannergooding]

Also ensured we had scenarios for all methods in the Vector64<T>, Vector128<T>, and Vector256<T> structs to ensure they throw NotSupportedException for unsupported T.

• This isn't done "ideally" and adds the scenarios to the root template, which means that it runs as part of each generated test when we really only need to run them once... Going to try and pull them out into their own template and generate test instead. Fixed to be part of a standalone template/project

[tannergooding]

Something broken in ARM64 (CC. @CarolEidt):

Assert failure(PID 20706 [0x000050e2], Thread: 20706 [0x50e2]): Assertion failed '(varDsc->lvIsParam && !varDsc->lvIsRegArg) || isPrespilledArg' in 'System.Runtime.Intrinsics.Vector64`1[Double][System.Double]:ToVector128():struct:this' (IL size 36)

Noting that the current code is just doing the following and I'm working on getting a JIT Dump

        public Vector128<T> ToVector128()
        {
            ThrowIfUnsupportedType();
            Vector128<T>.ThrowIfUnsupportedType();

            Vector128<T> result = Vector128<T>.Zero;
            Unsafe.As<Vector128<T>, Vector64<T>>(ref result) = this;
            return result;
        }

[fiigii]

LGTM, thanks!

[fiigii]

Something broken in ARM64

I guess this failure is unrelated to this PR, so we can track it in another issue.

[CarolEidt]

Something broken in ARM64

I guess this failure is unrelated to this PR, so we can track it in another issue.

I'm not seeing this same failure in recent runs - they are either infrastructure failures or they pass. We should figure out why this is only failing on this with this PR.

[fiigii]

they are either infrastructure failures or they pass. We should figure out why this is only failing on this with this PR.

Yes, let's take a look at the JIT dump at first.

[tannergooding]

We should figure out why this is only failing on this with this PR.

Because the relevant code wasn't being compiled by the JIT before this PR (no tests/etc covered this code).

• It is also on failing for no_tiered_compilation

[tannergooding]

JitDump is here: JitDump.txt

I'm only able to get the issue to repro on Checked Arm/Arm64 (both Linux and Windows, including the AltJit). It only repros with TieredCompilation disabled.

[tannergooding]

The minimal repro I have gotten so far is:

        public static void Main(string[] args)
        {
            Vector128<double> x = default(Vector64<double>).ToVector128();
        }

Where ToVector128 is:

        public Vector128<T> ToVector128()
        {
            Vector128<T> result = Vector128<T>.Zero;
            Unsafe.As<Vector128<T>, Vector64<T>>(ref result) = this;
            return result;
        }

It only occurs for double, long, and ulong (not for float or any of the 6 other integer types).

Here is another, slightly smaller JIT Dump, with the throw helpers removed:
JitDump2.txt

[tannergooding]

Also worth noting is that this appears to be an issue with the return value of the Unsafe.As<Vector128<T>, Vector64<T>>(ref result) call.

Changing the code to the following removes the assert:

        public Vector128<T> ToVector128()
        {
            Vector128<T> result = Vector128<T>.Zero;
            ref Vector64<T> temp = ref Unsafe.As<Vector128<T>, Vector64<T>>(ref result);
            temp = this;
            return result;
        } 

[tannergooding]

(rebased the code, no other changes)

[CarolEidt]

AFAICT from the dump, it seems that things are going wrong in the morphing of the assignment to the lower 8 bytes of result. At that point, I believe that fgMorphCopyBlock should be marking it as address taken since it's a partial assignment. But it's a mystery why this would only be an issue for the double variant.

[CarolEidt]

So, given that this is an arm64-only issue, and that it's only in the new tests, I guess the right thing to do is to exclude them for arm64 and open an issue to track fixing it.

[tannergooding]

So, given that this is an arm64-only issue

Just to clarify, it occurs on both ARM and ARM64 and for the double, long, and ulong variants. I have logged https://github.com/dotnet/coreclr/issues/21064 and pushed an additional commit that disables the relevant tests.

[tannergooding]

@jashook, what is the current "correct" way to exclude a particular test on ARM/ARM64? We seem to have a few files (including failingTests, issues.targets, and Tests.lst; and it isn't clear which ones need touching (was looking at https://github.com/dotnet/coreclr/blob/master/Documentation/building/test-configuration.md#adding-test-guidelines).

[tannergooding]

Changed just issues.targets for the time being, as that appears to be the appropriate location (Tests.lst are marked as produced automatically).

[tannergooding]

Hmm, that doesn't appear to work either

[jashook]

Issues.targets should be correct. I can’t take a look atm but when I’m back near a machine I can help out. Prolly a 2 hr eta.

[tannergooding]

I seem to have figured it out, even though the include has a * in the name, it seems you can't do:
$(XunitTestBinBase)/JIT/HardwareIntrinsics/General/Vector64_1/*

To cover both:

• $(XunitTestBinBase)/JIT/HardwareIntrinsics/General/Vector64_1/Vector64_1_r/*
• $(XunitTestBinBase)/JIT/HardwareIntrinsics/General/Vector64_1/Vector64_1_ro/*

and you instead have to list out the folder that contains the test files directly.