whisper.cpp
build : detect AVX512 in Makefile, add AVX512 option in CMake
#2043
Merged
build : detect AVX512 in Makefile, add AVX512 option in CMake #2043
make : add AVX512 detection to Makefile and CMakeLists.txt
make : autodetect more AVX512 instruction subsets
cmake : do not default to AVX512, must be enabled explicitly
cmake : enable a set of AVX512 subsets, when AVX512 is turned on
| 61 | 61 | ||
| 62 | option(WHISPER_NO_AVX "whisper: disable AVX" OFF) | ||
| 63 | option(WHISPER_NO_AVX2 "whisper: disable AVX2" OFF) | ||
| 64 | option(WHISPER_NO_FMA "whisper: disable FMA" OFF) | ||
| 65 | option(WHISPER_NO_F16C "whisper: disable F16c" OFF) | ||
| 62 | option(WHISPER_NO_AVX "whisper: disable AVX" OFF) | ||
| 63 | option(WHISPER_NO_AVX2 "whisper: disable AVX2" OFF) | ||
| 64 | option(WHISPER_AVX512 "whisper: enable AVX512" OFF) |
przemoc1 year agoI suggest using WHISPER_NO_AVX512 to be consistent with rest of these settings (until they will be overhauled one day in whisper.cpp to use positive notion instead of negative one), and naturally changing default to ON in such case.
Sorry if my rant in side note made you misunderstand the expectation for this PR.
didzis1 year agoAre you suggesting to use WHISPER_NO_AVX512 set to ON by default?
przemoc1 year agoYes, to be consistent, because all other ISA extension options in whisper.cpp have such polarity.
(Maybe we'll change it in the future, but if it will happen, then it should be done separately.)
| 144 | 144 | CXXFLAGS += -mavx2 | |
| 145 | 145 | endif | |
| 146 | 146 | ||
| 147 | AVX512F_M := $(shell $(CPUINFO_CMD) | grep -iw 'AVX512F') | ||
| 148 | ifneq (,$(AVX512F_M)) | ||
| 149 | CFLAGS += -mavx512f | ||
| 150 | CXXFLAGS += -mavx512f | ||
| 151 | endif | ||
| 152 | |||
| 153 | AVX512DQ_M := $(shell $(CPUINFO_CMD) | grep -iw 'AVX512DQ') | ||
| 154 | ifneq (,$(AVX512DQ_M)) | ||
| 155 | CFLAGS += -mavx512dq | ||
| 156 | CXXFLAGS += -mavx512dq | ||
| 157 | endif | ||
| 158 | |||
| 159 | AVX512CD_M := $(shell $(CPUINFO_CMD) | grep -iw 'AVX512CD') | ||
| 160 | ifneq (,$(AVX512CD_M)) | ||
| 161 | CFLAGS += -mavx512cd | ||
| 162 | CXXFLAGS += -mavx512cd | ||
| 163 | endif | ||
| 164 | |||
| 165 | AVX512CD_M := $(shell $(CPUINFO_CMD) | grep -iw 'AVX512BW') | ||
| 166 | ifneq (,$(AVX512BW_M)) | ||
| 167 | CFLAGS += -mavx512bw | ||
| 168 | CXXFLAGS += -mavx512bw | ||
| 169 | endif | ||
| 170 | |||
| 171 | AVX512VL_M := $(shell $(CPUINFO_CMD) | grep -iw 'AVX512VL') | ||
| 172 | ifneq (,$(AVX512VL_M)) | ||
| 173 | CFLAGS += -mavx512vl | ||
| 174 | CXXFLAGS += -mavx512vl | ||
| 175 | endif |
przemoc1 year agoI suggest combining them into one block, where we check just for AVX512F, but then add flags related to all 5 AVX-512 subsets (F, CD, VL, DQ, BW). I don't believe Xeon Phi existence is good enough reason check all of these subsets individually. I never suggested checking them individually.
Table at https://en.wikipedia.org/wiki/AVX-512#CPUs_with_AVX-512 explains why.
Let's have a simpler code. If there will be mainstream CPU in the future not fitting into the assumption (F presence means presence of CD, VL, DQ and BW too), then we can start playing again with checking those flags individually.
| 174 | CXXFLAGS += -mavx512vl | ||
| 175 | endif | ||
| 176 | |||
| 177 | AVX512VNNI_M := $(shell $(CPUINFO_CMD) | grep -iwE 'AVX512_VNNI|AVX512VNNI') |
przemoc1 year agoAVX512_VNNI is for Linux, so just out of curiosity:
Which OS uses AVX512VNNI?
As you're adding VNNI beside basic AVX-512 subsets, I would suggest adding also VBMI, as llama.cpp does it too. It's nice to have some kind of parity between these projects.
Moreover, if you're adding VNNI and VBMI in Makefile, you should add them in CMakeLists.txt too, via WHISPER_NO_AVX512_VNNI and WHISPER_NO_AVX512_VBMI, defaulting to ON. I suggest using add_compile_definitions similarly like llama.cpp does.
didzis1 year agoAdded AVX512VNNI for pure symmetry just in case any system does have it.
przemoc1 year agoI asked about AVX512VNNI, didn't request removing. But I see you removed it in dc69460.
So I looked around and it seems AVX512VNNI is the name reported by FreeBSD.
http://fxr.watson.org/fxr/source/x86/x86/identcpu.c#L994
Please bring back grep -iwE 'AVX512_VNNI|AVX512VNNI' for VNNI detection.
didzis1 year agoGreat, my feeling was right then.
make : consolidate AVX512 subsets, add AVX512 VBMI
cmake : revert to NO AVX512 setting, add settings for AVX512 VNNI andβ¦
make : re-introduce AVX512VNNI back
cmake : remove superfluous comment line
didzis
changed the title 
Login to write a write a comment.
Adds missing AVX512 detection to Makefile and CMakeLists.txt