Repository navigation
matrix4x4createfromaxisangletest failing on NativeAOT #72149
Description
Activity
- changed the title
[-]system.numerics.tests.matrix4x4tests.matrix4x4createfromaxisangletest[/-][+]matrix4x4createfromaxisangletest failing on NativeAOT[/+]on Jul 14, 2022 Deleted duplicates in runfo, didn't mean to close this
46 remaining items
Given this is NAOT only and NAOT must target a lower baseline, its possible there is a bug in the
dppsemulation emitted for SSE2 only hardware that isn't correctly accounting for this.I'll take a look tomorrow.
Given this is NAOT only and NAOT must target a lower baseline
If we don't have any other test coverage for the lower baselines, it might be a good quality week topic. With NAOT one at least has a deterministic repro once a bad binary has been produced. Running into this in a JIT configuration would be a lot more difficult to root cause.
This is not the first issue caused by this test hole - there were #72081 and #72158 just in the past month. NAOT covers the lowest denominator but there's a whole spectrum in between the lowest denominator and whatever machines we run the tests on that can happen in the real world and it looks like we don't test.
We do test, but only in the outer loop and only for the runtime tests. There has been a longstanding issue to also run the library tests under the various JIT Stress options for ISA enablement
Given this is NAOT only and NAOT must target a lower baseline, its possible there is a bug in the
dppsemulation emitted for SSE2 only hardware that isn't correctly accounting for this.I'll take a look tomorrow.
Assigning to Tanner.
@tannergooding Did you have a chance to take a look?
Yes. but it doesn't look to be in the
Vector3.Dotemulation.The relevant logic is here: https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/lowerxarch.cpp#L3350
We pick
Sse.Multiply,Sse3.HorizontalAdd-or-Sse.Shuffle, andSse.Addand then the very first thing we do is mask things so that the unused component is zero: https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/lowerxarch.cpp#L3418- Notably we could do this "more efficiently" now that we have
GT_CNS_VEC, but that's an unrelated JIT throughput optimization
- Notably we could do this "more efficiently" now that we have
The called Matrix and Quaternion methods aren't accelerated, just the
Vector3.Normalizecall, the various field accesses, theUnitX/Y/Zproperty accesses, and theVector3constructor call.The test doesn't really use any special tricks or code paths either:
Here is codegen for
Vector3.Lengththat I believe is the root cause of the problem:[MethodImpl(MethodImplOptions.NoInlining)] static float f(Vector3 v) => v.Length();movss xmm0,dword ptr [rcx+8] ds:00000052`9e6ff828=3f800000 movsd xmm1,mmword ptr [rcx] shufps xmm1,xmm0,44h // xmm1 has Vector3. The top 4 bytes of xmm1 have undefined value. They happen to be well defined in this specific case, but the JIT assumes the general case where they are undefined. movups xmm0,xmmword ptr [xxx!_readonlydata_xxx_Program____Main___g__f_0_0 (00007ff6`59060ee0)] // 0x0000 FFFF FFFF FFFF - mask for the top 4 bytes andps xmm0,xmm1 // xmm0 has properly masked Vector3 mulps xmm0,xmm1 // We are multiplying properly masked Vector3 with unmasked Vector3! movaps xmm1,xmm0 shufps xmm1,xmm0,0B1h addps xmm1,xmm0 movaps xmm0,xmm1 shufps xmm0,xmm1,4Eh addps xmm0,xmm1 sqrtss xmm0,xmm0 retI think
mulps xmm0,xmm1is wrong. It should bemulps xmm0,xmm0so that we multiply the masked values. The bad code works most of the time (0 multiplied by undefined value is zero) except when the undefined value happens to be NaN that will turn the whole thing into NaN.Does this analysis make sense? Can you take it from here?
Yes, that makes sense. It should definitely be multiplying
tmp * tmp, rather thantmp * original.I'll see about getting a fix up first thing tomorrow.
- ghost locked as resolved and limited conversation to collaborators
on Oct 2, 2022
Metadata
Metadata
Assignees
Type
Projects
- Status
Frequency - last 30 days as of 8/27:
Note: main branch failures in last 30 days
Runfo report