1871151 - Crash in [@ gemmology::(anonymous namespace)::kernel::maddw]

Status: VERIFIED FIXED

(Firefox :: Translations, defect)

Description

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]:

Crash report: https://crash-stats.mozilla.org/report/index/7c105665-3c0d-461a-a614-b4baa0231220

Reason: EXCEPTION_ILLEGAL_INSTRUCTION

Top 10 frames of crashing thread:

0  xul.dll  gemmology::  third_party/gemmology/gemmology.h:208
0  xul.dll  gemmology::  third_party/gemmology/gemmology.h:640
0  xul.dll  gemmology::  third_party/gemmology/gemmology.h:646
0  xul.dll  gemmology::Engine<xsimd::avxvnni>::Shift::PrepareBias<gemmology::callbacks::UnquantizeAndAddBiasAndWrite>  third_party/gemmology/gemmology.h:1303
1  xul.dll  js::intgemm::IntrI8PrepareBias::<lambda_4>::operator const  js/src/intgemm/IntegerGemmIntrinsic.cpp:317
1  xul.dll  xsimd::detail::dispatcher<`lambda at /builds/worker/checkouts/gecko/js/src/intgemm/IntegerGemmIntrinsic.cpp:317:3', xsimd::arch_list<xsimd::avxvnni, xsimd::avx2, xsimd::ssse3, xsimd::sse2> >::walk_archs  third_party/xsimd/include/xsimd/config/xsimd_arch.hpp:238
1  xul.dll  xsimd::detail::dispatcher<`lambda at /builds/worker/checkouts/gecko/js/src/intgemm/IntegerGemmIntrinsic.cpp:317:3', xsimd::arch_list<xsimd::avxvnni, xsimd::avx2, xsimd::ssse3, xsimd::sse2> >::operator  third_party/xsimd/include/xsimd/config/xsimd_arch.hpp:253
1  xul.dll  js::intgemm::IntrI8PrepareBias  js/src/intgemm/IntegerGemmIntrinsic.cpp:301
2  ?  @0x00000176f245301e  
3  xul.dll  WasmMemoryCopy  js/src/wasm/WasmInstance.cpp:566

Looks like a regression from bug 1868949.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1868949

:sergesanspaille, since you are the author of the regressor, bug 1868949, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[[:sergesanspaille]]

This probably means that the runtime detection code is incorrect :-/

Comment 3

[[:sergesanspaille]]

Based on the crash report, the proc is a Tiger Lake, which supports avx vnni. I've double checked- the detection code and it looks correct. And the stack trace points at vpdpbusd which is indeed an AVX VNNI instruction :-/

Comment 4

[Gian-Carlo Pascutto [:gcp]]

Serge, Tiger Lake supports AVX512 VNNI instructions (in 512-bit and 256-bit width), but that's not the same as AVXVNNI on later CPUs like Alder Lake, which have a different VEX prefix. When you see vpdpbusd, check whether the instruction prefix is correct, specifically, not VEX for Tiger Lake.

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Yannis confirmed that the disassembly shows vex vpdpbusd, so it's AVXVNNI, but it means the detection code misfired. As far as I can tell you're checking the right bits though.

Comment 6

[Gian-Carlo Pascutto [:gcp]]

This reproduces on my Zen 4: https://crash-stats.mozilla.org/report/index/ba465ecd-1ccf-425a-84d7-063db0231221

Comment 7

[Gian-Carlo Pascutto [:gcp]]

I'm looking through the code, and there was a suspicion "best" CPU detection was at fault (https://github.com/xtensor-stack/xsimd/blob/a48ab430d4b84ecd5449180ee1c6d2eed67c4191/include/xsimd/config/xsimd_cpuid.hpp#L189), but I don't see anything wrong there. Note that even if AVXVNNI detection misfires, it should be overruled by the AVX512_VNNI detection that follows.

What I do notice is that gemmology (https://github.com/mozilla/gemmology/blob/40dda91e99088ff80e21d71e57415aa491a0954c/gemmology.h#L208) ONLY has code for the AVXVNNI version, not the AVX512_VNNI one. So indeed misfiring detection (rather than "best") could still be the cause.

I'm still looking, but for example this looks like a minor bug (wouldn't cause this crash tho as we don't compile with Intel): https://github.com/xtensor-stack/xsimd/blob/a48ab430d4b84ecd5449180ee1c6d2eed67c4191/include/xsimd/config/xsimd_cpuid.hpp#L116

And this also looks suspicious, but it's probably dead code for Firefox: https://searchfox.org/mozilla-central/source/mozglue/misc/SSE.cpp#65 (comment that follows is also misleading)

Comment 8

[Gian-Carlo Pascutto [:gcp]]

Theory:
https://searchfox.org/mozilla-central/source/third_party/xsimd/include/xsimd/config/xsimd_arch.hpp#245

On AVX512 machines, this would set best_arch_found(available_architectures().best) to some level of AVX512 support, e.g. generic::version(3, 4, 1);. Wouldn't the following code: https://hg.mozilla.org/mozilla-central/file/37657c7691664026e54babf7d1cf608fe58a92fb/third_party/xsimd/include/xsimd/config/xsimd_arch.hpp#l237
then match on AVXVNNI generic::version(2, 3, 0)?

This seems to match the specific case we have here where higher version hardware support doesn't imply lower version support, and we provide a lower version routine, but not the higher version one.

Comment 9

[[:sergesanspaille]]

Upstream patch: https://github.com/xtensor-stack/xsimd/pull/994.patch

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The bug is marked as tracked for firefox123 (nightly). However, the bug still isn't assigned.

:marco, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 11

[Marco Castelluccio [:marco]]

Serge is working on it.

Comment 12

[[:sergesanspaille]]

Attachment: Bug 1871151 - Backport xsimd dispatch mechanism fix r=gcp

See upstream https://github.com/xtensor-stack/xsimd/commit/bc29d96401b8c3cb8b120d392bc5b7ce494ff1f4

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd8dc9b1338d
Backport xsimd dispatch mechanism fix r=gcp

Comment 14

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/cd8dc9b1338d

Comment 15

[Marco Castelluccio [:marco]]

:gcp, could you confirm the fix?

Comment 16

[Gian-Carlo Pascutto [:gcp]]

Working on Zen 4.