search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.29k stars 4.74k forks source link

AF: ILC: Assertion failed '((simdSize == 16) && compOpportunisticallyDependsOn(InstructionSet_SSE41)) || ((simdSize == 32) && compOpportunisticallyDependsOn(InstructionSet_AVX2))' #109760

Open jkotas opened 1 day ago

jkotas commented 1 day ago

/azp run runtime-nativeaot-outerloop fails during compilation of System.Numerics.Vectors.Tests with:

ILC: Assertion failed '((simdSize == 16) && compOpportunisticallyDependsOn(InstructionSet_SSE41)) || ((simdSize == 32) && compOpportunisticallyDependsOn(InstructionSet_AVX2))' in 'System.Numerics.Vector`1[long]:System.Runtime.Intrinsics.ISimdVector<System.Numerics.Vector<T>,T>.MultiplyAddEstimate(System.Numerics.Vector`1[long],System.Numerics.Vector`1[long],System.Numerics.Vector`1[long]):System.Numerics.Vector`1[long]' during 'Importation' (IL size 9; hash 0x709b06b8; FullOpts)

Fails on Windows and Linux x64. Example of a log: https://dev.azure.com/dnceng-public/public/_build/results?buildId=867214&view=logs&jobId=7e7cde7f-4669-5f41-976b-370a74291e60&j=7e7cde7f-4669-5f41-976b-370a74291e60&t=9ee39664-b162-579c-6968-bf80271a4c1d

dotnet-policy-service[bot] commented 1 day ago

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch See info in area-owners.md if you want to be subscribed.

jkotas commented 1 day ago

Regression introduced by #106594 . cc @tannergooding

tannergooding commented 1 day ago

At first glance it looks like a path getting NAOT is opting to emit Add(Mul(a, b), c) which should also be reproing in the ISA stress jobs for V128/V256, but it looks like it isn't because those the relevant tests live in the libraries side and not the JIT tests side.

We used to exit early for SIMD GT_MUL involving long/ulong on older hardware, but SSE4.1 fallback acceleration support was added back around July, and it looks like updating the Multiply was updated but not MultiplyAddEstimate

Fix should just be mirroring the GT_MUL guard in MultiplyAddEstimate, so I'll get that up early tomorrow.