[Test Failure] the Sse2.MultiplyLow test fails for some inputs.

[tannergooding]

See https://ci.dot.net/job/dotnet_coreclr/job/master/job/jitstress/job/x86_checked_windows_nt_jitx86hwintrinsicnoavx_prtest/56/, for an example.

[tannergooding]

FYI. @4creators

[tannergooding]

In the validation logic the test is doing:

CheckMethod<short> checkInt16 = (short x, short y, short z, ref short a) =>
{
    var tmp = ((int)x * y) * 0x0000ffff;
    a = unchecked((short)tmp);
    return -a == z;
};

@4creators, could you explain the reasoning behind the * 0x0000ffff?

The architecture manual indicates the logic is simply:

So I would think we just want to do:

var expectedResult = unchecked((short)((int)(x) * (int)(y)));
return expectedResult == z;

[4creators]

could you explain the reasoning behind the * 0x0000ffff?

Typo - should be & 0x0000ffff - unlucky that it worked for same values though - preparing fix

[tannergooding]

should be & 0x0000ffff

The unchecked((short)(tmp)) should handle that Edit: Nevermind, I was thinking of a conversion to ushort, short may still need the &

What about the logic for -a == z;? (not sure why a is being negated).

[4creators]

What about the logic for -a == z;? (not sure why a is being negated).

Suspect that it could be merge problem during rebasing - I have started with > 50 commits before rebasing first to 5 and finally to 1 commit

[CarolEidt]

@tannergooding @fiigii @4creators - The fact that this test was exhibiting non-deterministic behavior is problematic. I believe that it is still the plan to change the Sse2 tests to use the new templates, but if that's not happening soon, we should change TestTableSse2.cs to not use DateTime.UtcNow.Ticks as an argument to Random.

[tannergooding]

The fact that this test was exhibiting non-deterministic behavior is problematic.

Agreed.

I believe that it is still the plan to change the Sse2 tests to use the new templates, but if that's not happening soon

I was planning on moving most of the tests over after dotnet/coreclr#16095 goes in.

Then I was going to work on adding templates for testing commutative operations and containment tests for LoadScalar

[CarolEidt]

Thanks @tannergooding!

[4creators]

@CarolEidt @tannergooding In such case I will speed up test conversion to common template and fix

TestTableSse2.cs to not use DateTime.UtcNow.Ticks as an argument to Random

[4creators]

@tannergooding Which part of tests should I port to common template?

[tannergooding]

In such case I will speed up test conversion to common template and fix

I was explicitly waiting on dotnet/coreclr#16095 in order to help prevent merge conflicts and to reduce the total number of changes, since it updates the tests to include additional scenarios.

Which part of tests should I port to common template?

All of it. The only thing the Sse2 tests are doing that the template is not is validating loops, which I was going to update the template to cover (one commit moves the tests over to the template, a second commit updates the template to test loops and regenerates all the tests).

[4creators]

OK than I will start work once dotnet/coreclr#16095 will be in except for parts which do not have to wait.

[tannergooding]

OK than I will start work once dotnet/coreclr#16095 will be in except for parts which do not have to wait.

I already have part of this done locally (on top of my existing changes), so that might just end up duplicating work.

It would be beneficial to have any new tests using the template from the get-go (if you plan on resolving the merge conflicts and continuing with dotnet/coreclr#15777, for example).