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

importer does not correctly mark the HWIntrinsic SIMD types as SIMD types when inlining #9675

Closed
tannergooding opened this issue Feb 6, 2018 · 11 comments · Fixed by dotnet/coreclr#16365
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Milestone

Comments

@tannergooding
Copy link
Member

This is currently failing several of the jitstress jobs.

Having dug into this briefly, it looks like impInlineInitVars (and possibly other locations as well) are only checking for SIMD types via isSIMDClass (which itself only checks for isInSIMDModule()).

This causes inlining to fail various assertions later on (stloc, for example, will fail when it gets a TYP_SIMD to TYP_STRUCT comparison).

This also makes the check now invalid for the recently moved Vector<T> type.

@tannergooding
Copy link
Member Author

FYI. @CarolEidt, @fiigii, @eerhardt, @jkotas

@tannergooding
Copy link
Member Author

I will try to take a closer look tonight, when I have some time.

@fiigii
Copy link
Contributor

fiigii commented Feb 6, 2018

Maybe we can change Vector<T> recognition to [Intrinsic] at first, then isSIMDClass can be unified for Vector<T> and Vector64/128/256<T>.

@tannergooding
Copy link
Member Author

I think that is probably the correct approach, there is several related TODOs in the codebase indicating something similar.

@fiigii
Copy link
Contributor

fiigii commented Feb 6, 2018

change Vector recognition to [Intrinsic] at first,

Let me make this change.

@tannergooding
Copy link
Member Author

@fiigii, I've moved this into the Hardware Intrinsics project and assigned it to you, since you are actively workin on it.

@fiigii
Copy link
Contributor

fiigii commented Feb 8, 2018

@tannergooding Thank you!

@tannergooding
Copy link
Member Author

@fiigii, has there been any progress here? There have been a few duplicate bugs logged against this now.

@fiigii
Copy link
Contributor

fiigii commented Feb 13, 2018

@tannergooding I was working on dotnet/coreclr#16183 yesterday. I will look into this one today (it looks like urgent also), sorry to the delay.

fiigii referenced this issue in fiigii/coreclr Feb 13, 2018
fiigii referenced this issue in fiigii/coreclr Feb 13, 2018
@BruceForstall
Copy link
Member

BruceForstall commented Feb 13, 2018

Is this what is causing the following failures in JitStress2 (for example):

JIT_HardwareIntrinsics._X86_Sse41_Sse41_ro_Sse41_ro_._X86_Sse41_Sse41_ro_Sse41_ro_cmd
JIT_HardwareIntrinsics._X86_Sse42_Sse42_ro_Sse42_ro_._X86_Sse42_Sse42_ro_Sse42_ro_cmd
JIT_HardwareIntrinsics._X86_Avx_Avx_ro_Avx_ro_._X86_Avx_Avx_ro_Avx_ro_cmd
JIT_HardwareIntrinsics._X86_Sse2_Sse2_ro_Sse2_ro_._X86_Sse2_Sse2_ro_Sse2_ro_cmd
JIT_HardwareIntrinsics._X86_Avx2_Avx2_ro_Avx2_ro_._X86_Avx2_Avx2_ro_Avx2_ro_cmd
JIT_HardwareIntrinsics._X86_Sse_Sse_ro_Sse_ro_._X86_Sse_Sse_ro_Sse_ro_cmd
        Assert failure(PID 10572 [0x0000294c], Thread: 13988 [0x36a4]): Assertion failed 'genActualType(lclTyp) == genActualType(op1->gtType) || genActualType(lclTyp) == TYP_I_IMPL && op1->IsVarAddr() || (genActualType(lclTyp) == TYP_I_IMPL && (op1->gtType == TYP_BYREF || op1->gtType == TYP_REF)) || (genActualType(op1->gtType) == TYP_I_IMPL && lclTyp == TYP_BYREF) || (varTypeIsFloating(lclTyp) && varTypeIsFloating(op1->TypeGet())) || ((genActualType(lclTyp) == TYP_BYREF) && genActualType(op1->TypeGet()) == TYP_REF) : Possibly bad IL with CEE_stloc.0 at offset 0011h (op1=simd16 op2=NULL stkDepth=0)' in 'JIT.HardwareIntrinsics.X86.Program:CompareEqualUInt64()' (IL size 114)
        
            File: d:\j\workspace\x64_checked_w---8a0029a8\src\jit\importer.cpp Line: 10729
            Image: D:\j\workspace\x64_checked_w---8a0029a8\bin\tests\Windows_NT.x64.Checked\Tests\Core_Root\CoreRun.exe

https://ci.dot.net/job/dotnet_coreclr/job/master/job/jitstress/job/x64_checked_windows_nt_jitstress2/427/#showFailuresLink

https://ci.dot.net/job/dotnet_coreclr/job/master/job/jitstress/job/x64_checked_windows_nt_jitstress1/430/

Or do we need a new issue for those failures?

If this is the same issue, please make fixing it a priority; it is causing many jobs to be "red" (run with failures).

@fiigii
Copy link
Contributor

fiigii commented Feb 13, 2018

@BruceForstall this will be fixed by dotnet/coreclr#16365, but another failure appears...

fiigii referenced this issue in fiigii/coreclr Feb 13, 2018
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants