Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VectorSingle and VectorDoubleEqualsNonCanonicalNaNTest fails on Mono #74781

Closed
steveisok opened this issue Aug 29, 2022 · 13 comments
Closed

VectorSingle and VectorDoubleEqualsNonCanonicalNaNTest fails on Mono #74781

steveisok opened this issue Aug 29, 2022 · 13 comments
Assignees
Labels
area-Codegen-meta-mono disabled-test The test is disabled in source code against the issue runtime-mono specific to the Mono runtime
Milestone

Comments

@steveisok
Copy link
Member

steveisok commented Aug 29, 2022

From the logs, the Assert.False is failing

Assert.False(new Vector<float>(i) == new Vector<float>(j));

It is tough to tell which nan value is the cause:

float.CopySign(float.NaN, -0.0f), // -qnan same as float.NaN
float.CopySign(float.NaN, +0.0f), // +qnan
float.CopySign(snan, -0.0f), // -snan
float.CopySign(snan, +0.0f), // +snan

Stack trace:

Assert.False() Failure\nExpected: False\nActual:   True
at System.Numerics.Tests.GenericVectorTests.VectorSingleEqualsNonCanonicalNaNTest() in /_/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs:line 893
   at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)]]
Assert.False() Failure\nExpected: False\nActual:   True
at System.Numerics.Tests.GenericVectorTests.VectorDoubleEqualsNonCanonicalNaNTest() in /_/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs:line 869
   at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)]]

Report

Summary

24-Hour Hit Count 7-Day Hit Count 1-Month Count
0 0 0
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 29, 2022
@steveisok steveisok added blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms' and removed area-System.Numerics untriaged New issue has not been triaged by the area owner labels Aug 29, 2022
@ghost
Copy link

ghost commented Aug 29, 2022

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

From the logs, the Assert.False is failing

Assert.False(new Vector<float>(i) == new Vector<float>(j));

It is tough to tell which nan value is the cause:

float.CopySign(float.NaN, -0.0f), // -qnan same as float.NaN
float.CopySign(float.NaN, +0.0f), // +qnan
float.CopySign(snan, -0.0f), // -snan
float.CopySign(snan, +0.0f), // +snan

Stack trace:

Assert.False() Failure\nExpected: False\nActual:   True
at System.Numerics.Tests.GenericVectorTests.VectorSingleEqualsNonCanonicalNaNTest() in /_/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs:line 893
   at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)]]
Assert.False() Failure\nExpected: False\nActual:   True
at System.Numerics.Tests.GenericVectorTests.VectorDoubleEqualsNonCanonicalNaNTest() in /_/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs:line 869
   at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)]]
Author: steveisok
Assignees: -
Labels:

area-System.Numerics, untriaged

Milestone: -

@ghost
Copy link

ghost commented Aug 29, 2022

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

From the logs, the Assert.False is failing

Assert.False(new Vector<float>(i) == new Vector<float>(j));

It is tough to tell which nan value is the cause:

float.CopySign(float.NaN, -0.0f), // -qnan same as float.NaN
float.CopySign(float.NaN, +0.0f), // +qnan
float.CopySign(snan, -0.0f), // -snan
float.CopySign(snan, +0.0f), // +snan

Stack trace:

Assert.False() Failure\nExpected: False\nActual:   True
at System.Numerics.Tests.GenericVectorTests.VectorSingleEqualsNonCanonicalNaNTest() in /_/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs:line 893
   at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)]]
Assert.False() Failure\nExpected: False\nActual:   True
at System.Numerics.Tests.GenericVectorTests.VectorDoubleEqualsNonCanonicalNaNTest() in /_/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs:line 869
   at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)]]
Author: steveisok
Assignees: -
Labels:

area-System.Numerics, blocking-clean-ci

Milestone: -

@steveisok
Copy link
Member Author

/cc @karelz

@akoeplinger
Copy link
Member

I think this fails on all Mono platforms/configs, we just didn't see it due to the issue that will be fixed by #74788.

I'll disable the tests for Mono for now.

@akoeplinger akoeplinger added runtime-mono specific to the Mono runtime disabled-test The test is disabled in source code against the issue and removed os-android x64 labels Aug 30, 2022
@akoeplinger akoeplinger changed the title [Android] VectorSingle and VectorDoubleEqualsNonCanonicalNaNTest fails on Android x64 [Android] VectorSingle and VectorDoubleEqualsNonCanonicalNaNTest fails on Mono Aug 30, 2022
@akoeplinger akoeplinger changed the title [Android] VectorSingle and VectorDoubleEqualsNonCanonicalNaNTest fails on Mono VectorSingle and VectorDoubleEqualsNonCanonicalNaNTest fails on Mono Aug 30, 2022
akoeplinger added a commit to radical/runtime that referenced this issue Aug 30, 2022
@tannergooding
Copy link
Member

Do we know why or how Mono is failing here?

@steveisok
Copy link
Member Author

Do we know why or how Mono is failing here?

Not yet. I was primarily trying to keep CI clean.

/cc @SamMonoRT

@MichalPetryka
Copy link
Contributor

Should we maybe add custom messages to the asserts that will say which nans are failing?

radical added a commit that referenced this issue Aug 30, 2022
* CI: Fix helix test results reporting

PR #73060 broke uploading of helix test results. This was caused by the
change:

```xml
      <HelixPostCommands>@(HelixPostCommand)</HelixPostCommands>
```

This is overwriting the existing value of `$(HelixPostCommands)`, which
gets set to have the upload script invocation in https://github.com/dotnet/arcade/blob/34dff939b4a91e4693f78a856e0e055c1a3f3fba/src/Microsoft.DotNet.Helix/Sdk/tools/azure-pipelines/AzurePipelines.MonoQueue.targets#L8-L15 at evaluation time.

Fix by *appending* to the property.

Thanks to ChadNedzlek for finding the cause!

Fixes #74699 .

* Disable failing tests, see #74781

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
@SamMonoRT SamMonoRT removed the blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms' label Aug 30, 2022
@SamMonoRT
Copy link
Member

removing blocking-clean-ci tag as tests have been disabled till we identify the cause/fix.

@fanyang-mono
Copy link
Member

@steveisok Do you happen to remember which platform the test failed?

@steveisok
Copy link
Member Author

@steveisok Do you happen to remember which platform the test failed?

Looks like they fail for all platforms.

@dakersnar dakersnar added area-Codegen-meta-mono untriaged New issue has not been triaged by the area owner and removed area-System.Numerics labels Sep 29, 2022
@dakersnar
Copy link
Contributor

dakersnar commented Sep 29, 2022

Is this the right area label for this issue?

@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Oct 27, 2022
@matouskozak
Copy link
Member

matouskozak commented Nov 1, 2022

The failing cases are when comparing the same elements with each other from the nans array using the == operator (return True instead of False). This issue is caused by MiniJIT OP_XEQUAL:

case OP_XEQUAL: {
int temp_reg1 = mono_alloc_ireg (cfg);
int temp_reg2 = mono_alloc_ireg (cfg);
NEW_SIMD_INS (cfg, ins, temp, OP_PCMPEQD, temp_reg1, ins->sreg1, ins->sreg2);
NEW_SIMD_INS (cfg, ins, temp, OP_EXTRACT_MASK, temp_reg2, temp_reg1, -1);
temp->type = STACK_I4;
NEW_INS (cfg, ins, temp, OP_COMPARE_IMM);
temp->sreg1 = temp_reg2;
temp->inst_imm = 0xFFFF;
temp->klass = ins->klass;
ins->opcode = OP_CEQ;
ins->sreg1 = -1;
ins->sreg2 = -1;
break;
}

I can confirm that the tests work correctly when using LLVM JIT or software fallback.

In current implementation OP_XEQUAL doesn't emit different instructions for different input types which is causing that floats and doubles are handled incorrectly.

@matouskozak
Copy link
Member

This issue is fixed by #77770.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Codegen-meta-mono disabled-test The test is disabled in source code against the issue runtime-mono specific to the Mono runtime
Projects
None yet
Development

No branches or pull requests

9 participants