libntl / ntl

269 stars 53 forks source link

CPU feature detection fails with NATIVE=off #22

Closed orlitzky closed 1 year ago

orlitzky commented 1 year ago

We've been debugging some build failures over at https://bugs.gentoo.org/815775 and have learned that some of the CPU feature tests (AVX, PCLMUL, AVX2, FMA and AES_NIAVX, PCLMUL, AVX2, FMA and AES_NI) return false negatives when building with NATIVE=off, i.e. without -march=native in CXXFLAGS.

For the moment I'll re-enable NATIVE=on, but that's not a perfect solution since not all compilers support it, and it can still be overriden by the user's CXXFLAGS.

victorshoup commented 1 year ago

I don’t see an easy way around this problem. The default behavior for most compilers is to target a very restricted subset of the Intel instruction set. You need to use some compiler flag to target the full instruction set.

On Jun 14, 2023, at 2:09 AM, Michael Orlitzky @.***> wrote:

 We've been debugging some build failures over at https://bugs.gentoo.org/815775 and have learned that some of the CPU feature tests (AVX, PCLMUL, AVX2, FMA and AES_NIAVX, PCLMUL, AVX2, FMA and AES_NI) return false negatives when building with NATIVE=off, i.e. without -march=native in CXXFLAGS.

For the moment I'll re-enable NATIVE=on, but that's not a perfect solution since not all compilers support it, and it can still be overriden by the user's CXXFLAGS.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

orlitzky commented 1 year ago

I know that it would be annoying to implement, but for the sake of argument, a perfect solution for us would be to have build flags that allow the user to override the detection of each CPU feature. The correct flag values could then be determined by looking at the CPUID, as in https://github.com/projg2/cpuid2cpuflags/, and specified by the user -- relieving NTL of the responsibility to detect them.

So, for example, a new flag:

FMA=on|off|auto (default: auto)

If I don't specify it, the default (auto) would continue to work as it does now. But if I know that my CPU has FMA support and if NTL can't detect it (due to missing CXXFLAGS=-march or some other reason), then I could pass FMA=on to DoConfig.

I think that would work in most cases, but some refactoring would be required in any tests that check not only for the CPU feature, but also some additional stuff. Here I have AVX2 in mind, which checks not only for AVX2 support in the CPU, but also the size of int/long and the FP precision.

victorshoup commented 1 year ago

Not sure if that will work. If the CPU has the feature but the compiler doesn’t think it does, I don’t think you’ll get the feature. There are more specific compiler flags you can pass to enable the feature.

On Jun 14, 2023, at 7:55 AM, Michael Orlitzky @.***> wrote:

 I know that it would be annoying to implement, but for the sake of argument, a perfect solution for us would be to have build flags that allow the user to override the detection of each CPU feature. The correct flag values could then be determined by looking at the CPUID, as in https://github.com/projg2/cpuid2cpuflags/, and specified by the user -- relieving NTL of the responsibility to detect them.

So, for example, a new flag:

FMA=on|off|auto (default: auto)

If I don't specify it, the default (auto) would continue to work as it does now. But if I know that my CPU has FMA support and if NTL can't detect it (due to missing CXXFLAGS=-march or some other reason), then I could pass FMA=on to DoConfig.

I think that would work in most cases, but some refactoring would be required in any tests that check not only for the CPU feature, but also some additional stuff. Here I have AVX2 in mind, which checks not only for AVX2 support in the CPU, but also the size of int/long and the FP precision.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.

orlitzky commented 1 year ago

You're right. I thought there were fallbacks for the builtins but it's not for all of them. My CPUs are too old to test most but e.g. PCLMUL does fail without the right compiler flags.

I'll close this now since the description is misleading. But in light of that, my one remaining suggestion would be to have NTL_ENABLE_AVX_FFT=on ignored when the corresponding CPU features are unavailable. Right now, the build fails if NTL_ENABLE_AVX_FFT=on but NTL doesn't like the CPU, making it hard for us to determine if it's safe to turn such a feature on.

Thanks for clarifying.

victorshoup commented 1 year ago

I see. The AVX based FFT is a somewhat experimental feature…On Jun 14, 2023, at 11:24 AM, Michael Orlitzky @.***> wrote: You're right. I thought there were fallbacks for the builtins but it's not for all of them. My CPUs are too old to test most but e.g. PCLMUL does fail without the right compiler flags. I'll close this now since the description is misleading. But in light of that, my one remaining suggestion would be to have NTL_ENABLE_AVX_FFT=on ignored when the corresponding CPU features are unavailable. Right now, the build fails if NTL_ENABLE_AVX_FFT=on but NTL doesn't like the CPU, making it hard for us to determine if it's safe to turn such a feature on. Thanks for clarifying.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>