xinghuayu007 commented on PR #8694: URL: https://github.com/apache/incubator-gluten/pull/8694#issuecomment-2655836597
> > We shouldn't add a new flag for AVX512 of simdjson. We should sue the same config. Can we use the same way as #8027? > > Yes, it would be better to set the option according to the detected CPU architecture, avx, avx512, etc. One gap is velox's helper function `get_cxx_flags` doesn't consider avx512. I reviewed the code of Velox, 'SIMDJSON_AVX512_ALLOWED' is a cmake option for simd-json and it's default value is 'ON'. Velox detects the CPU architecture, like arm64, avx, sse, aarch64 and chooses suitable instruction set. For a CPU which owns AVX instruction set but not AVX512, Velox still uses SIMDJSON_AVX512_ALLOWED=ON to compile, it is not correct. I think it is better to detect the CPU architecture and set the value of SIMDJSON_AVX512_ALLOWED in Gluten then passes it to Velox. Although modifying the code of Velox is more direct, but not convenient. If I have any mistake, please tell me, thanks. Detect CPU architecture in Velox: <img width="943" alt="1739434684024" src="https://github.com/user-attachments/assets/a81d8bb2-8191-4ea6-a3fc-e101eb94b2c5"; /> The default value of SIMDJSON_AVX512_ALLOWED in simd-json <img width="1058" alt="1739434748067" src="https://github.com/user-attachments/assets/bae50a93-35c3-400e-af31-8116aef3c52f"; /> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
