Update TargetHasAVXSupport to correctly check for AVX support by accounting for all ISAs and bits necessary

[tannergooding]

This should partiallly resolve #61471

[ghost]

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

Issue Details

---

This resolves #61471

Author:	tannergooding
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[tannergooding]

@jkotas, @davidwrighton, this is called from exactly one place in methodtablebuilder.cpp: https://github.com/dotnet/runtime/blob/main/src/coreclr/vm/methodtablebuilder.cpp#L1150-L1151.

• I think this is "one time" but it would be good if that could be confirmed so we know it doesn't need to be cached.

Some other questions:

• Do we know of any other places that have an AVX check that isn't going through void EEJitManager::SetCpuInfo()?
  • The bug indicates this failed on a machine with just SSSE3, so my guess is there is another method that needs updating
• What would the right .NET 6 servicing branch be that this should backport against?

[jkotas]

Instead of duplicating the logic here, would it be better to just call into JIT manager to get the InstructionSet_AVX bit from CPUCompileFlags?

This logic will get even more complex once we fix #60035

[tannergooding]

Probably, yes.

I assumed there was some existing reason for this separation. If there isn't, it's likely more trivial to just reuse what we already have computed (assuming its correctly taking into account things like crossgen flags, etc)

[jkotas]

Actually, we do call into the JIT manager right after calling TargetHasAVXSupport anyway that should take care of everything. I think that the TargetHasAVXSupport is unnecessary and can be just deleted.

It does not explain where the bug may be.

[tannergooding]

I think that the TargetHasAVXSupport is unnecessary and can be just deleted.

I think you may be right...

It does not explain where the bug may be.

AFAIK, all of the logic for tracking and checking per-ISA checks (even for crossgen) is supposed to be driven around the information set by void EEJitManager::SetCpuInfo() and so if Avx isn't set there; the comparison of "is the R2R code supported" should fail if the method is annotated as uses Avx.

I'll try and dig in a bit more, but @davidwrighton might be able to give direct pointers to the right checks to look at/debug through.

[rickbrew]

I've sent two more builds to my private testers, one compiled for SSE4.2 and another compiled for SSE2.

If the SSE4.2 build also crashes on the SSSE3 system, then the issue runs deeper. Should know relatively soon (the tester is in Belarus, not sure of time zone differences). I also sent these builds to Tanner, he says he has a Q6600 he can test on.

[rickbrew]

My tester confirmed that the SSE4.2 build does not work on the SSSE3 system. The SSE2 build is fine.

[tannergooding]

Closing this one because, as @jkotas indicated this actually should have no impact and is just an "early out" effectively.

We could probably just remove this function and should consider doing so when we find the actual issue. Given that SSE4.2 also has problems when run on a system without SSE4.2 support; this indicates a potentially deeper issue in the crossgen logic.