Fix #1111.

[ncruces]

I know this can't go in because of the cpuid dependency, but maybe it helps.

I can confirm all tests pass on Windows+WSL on the affected machine, and on Windows+macOS+Linux on other machines.

[codefromthecrypt]

thanks, @ncruces I think considering we wrote our own assembler, I think we can find a way to get similar code needed here (with unit tests). e.g. in our assembly we have compileXXX functions that could add instructions needed like https://github.com/aregm/cpuid/blob/master/cpuidlow_amd64.s

Thanks for making the change. To proceed I think ideally we can make the build fail first, then we can adapt in logic we need until it passes.

[ncruces]

A point I made on #1111 is that since this is AOT you can do the CPUID at compile time, because for this specific instruction, doing it at run-time might not make sense (querying the CPU might negate the performance benefit of using the instruction in the first place). I'm sure you wizards know this already.

But feel free to close the PR. The point of it was only to facilitate testing that it was indeed a CPU issue. I tested it on every HW/OS I have access to, and (now) also your CI.

[codefromthecrypt]

@ncruces thanks again for the help.

Basically you are saying that since we don't compile for other archs (only the current one), detecting the host cpuid is going to be correct for all module compilation.

So, the idea is that we could have detection implemented as a _amd64 file, for example leveraging a .s file as the dep is doing. We could store that as a constant in amd64Compiler, though for unit tests maybe a field is fine also.

As you also mentioned, we could just generally avoid the problematic instructions, but I don't personally know the impact of that. It is possible that impact could be evaluated with benchmarks, also, especially if we allow toggle of this for testing, the test even where LZCNTL and TZCNTL are fine can try wasm that would compile down to using them, without using them.

I'm going to hand this off to @evacchi to noodle on. If I mis-restated anything above lemme know!

[ncruces]

It needs to be a field, not a const. Or, even, you run the check every single time.

Basically by compile-time I mean load-time, when you AOT compile the WASM binary into ASM. Then, at “WASM-runtime” you can use the instruction without checking since it's the same CPU.

This means you can freely use instructions where just making the check negates the benefit.

[mathetake]

cool, let's implement cpuid.HasExtraFeature(cpuid.ABM) by ourselves. Assigning @evacchi

[ncruces]

New implementation should, I expect, be easier to follow.
Again, feel free to close at your convenience.

[mathetake]

with the CPUID, #580 this could also be resolved