Fix compilation with GCC and Clang #587

AngryLoki · 2024-04-09T18:16:02Z

At this moment intel-extension-for-pytorch build fails both with GCC 13.2.1 and Clang 17/18/19.

GCC build fails with:

failure to cast c10:Half to _Float16. While Clang-based compilers use operator float() in c10:Half and perform c10:Half->float->_Float16 conversion, gcc fails.
Similar failure in VecOps<__m512h>, refuses to cast float to _Float16 automatically.

Clang fails with few issues:

Compiler crashes with VLA+openmp combination, this was reported with a workaround at clang crash when build IPEX code. llvm/llvm-project#75428
-Wl,-Bsymbolic-functions should be a linker attribute; when applied to compiler, considered as "unused parameter" and fails due to -Werror -Wall combination.

Common issue:

extern int FLAGS_caffe2_log_level should not be redeclared, otherwise build fails with different redefinition when buildsystem follows contradictory recommendation to enable GLog and GFlags from Clarify use of GLog/GFlags pytorch/pytorch#44948

Warnings/misc:

In Clang -Wall overrides some previous -Wno-* (e. g. -Wno-unused-function -Wall), so it should precede other flags
Disabled few Clang warnings due to spam
Added f16 suffix in headers for f16 constants (spam in Clang/gcc)
Fixed infinite recursion in bool operator!=(const LlgaTensorDesc& desc)

jgong5

LGTM in general. I guess we need to have a validation to check if the changes still work fine with older compiler toolchains. cc @xuhancn to review and stamp on it.

jgong5 · 2024-04-09T23:56:53Z

cmake/cpu/BuildFlags.cmake

+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-ignored-attributes")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-pessimizing-move")
+
+  if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-vla-cxx-extension")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-deprecated-builtins")


can you comment on the build errors you found without these?

Clang-only, for -Wno-vla-cxx-extension, tons of warnings in all places:

/src/intel-extension-for-pytorch/csrc/cpu/aten/kernels/MergedEmbeddingBagKrnl.cpp:880:33: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension] 880 | scalar_t* val_ptr[world_size]; | ^~~~~~~~~~ /usr/include/ATen/Dispatch.h:797:46: note: expanded from macro 'AT_DISPATCH_INDEX_TYPES' 797 | at::ScalarType::Long, index_t, __VA_ARGS__)) | ^~~~~~~~~~~ /usr/include/ATen/Dispatch.h:70:12: note: expanded from macro 'AT_PRIVATE_CASE_TYPE_USING_HINT' 70 | return __VA_ARGS__(); \ | ^~~~~~~~~~~ /usr/include/ATen/Dispatch.h:221:7: note: expanded from macro 'AT_DISPATCH_SWITCH' 221 | __VA_ARGS__ \ | ^~~~~~~~~~~ note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)

Clang-only, for -Wno-deprecated-builtins all related to ROBIN_HOOD_IS_TRIVIALLY_COPYABLE (few warnings)

/src/intel-extension-for-pytorch/csrc/cpu/utils/robin_hood.h:1545:29: warning: builtin __has_trivial_copy is deprecated; use __is_trivially_copyable instead [-Wdeprecated-builtins] 1545 | Cloner<Table, IsFlat && ROBIN_HOOD_IS_TRIVIALLY_COPYABLE(Node)>()(o, *this); | ^ /src/intel-extension-for-pytorch/csrc/cpu/utils/robin_hood.h:218:47: note: expanded from macro 'ROBIN_HOOD_IS_TRIVIALLY_COPYABLE' 218 | #define ROBIN_HOOD_IS_TRIVIALLY_COPYABLE(...) __has_trivial_copy(__VA_ARGS__)

Clang-only, for -Wno-pessimizing-move, repeated many times for inner_product/deconv/matmul/tensor.hpp/etc.

/src/intel-extension-for-pytorch/third_party/ideep/include/ideep/operators/matmul.hpp:1028:23: warning: moving a temporary object prevents copy elision [-Wpessimizing-move] 1028 | param.primitive = std::move(super(param.pd)); | ^ /src/intel-extension-for-pytorch/third_party/ideep/include/ideep/operators/matmul.hpp:1028:23: note: remove std::move call here 1028 | param.primitive = std::move(super(param.pd)); | ^~~~~~~~~~ ~

GCC-only, for -Wno-ignored-attributes, tons of warnings in all places:

/src/intel-extension-for-pytorch/csrc/cpu/tpp/woq/vec.h:43:21: warning: ignoring attributes on template argument ‘__m512’ [-Wignored-attributes] 43 | struct VecOps<__m512> { | ^ /src/intel-extension-for-pytorch/csrc/cpu/tpp/woq/vec.h:228:33: warning: ignoring attributes on template argument ‘__m512’ [-Wignored-attributes] 228 | inline std::tuple<__m512, __m512> _vec_load_bfloat16_as_two_floats( | ^ /src/intel-extension-for-pytorch/csrc/cpu/tpp/woq/vec.h:228:33: warning: ignoring attributes on template argument ‘__m512’ [-Wignored-attributes] /src/intel-extension-for-pytorch/csrc/cpu/tpp/woq/vec.h:284:34: warning: ignoring attributes on template argument ‘VecArray<N, c10::BFloat16>::vec_type’ {aka ‘__m512’} [-Wignored-attributes] 284 | using vec_ops = VecOps<vec_type>; | ^ /src/intel-extension-for-pytorch/csrc/cpu/tpp/woq/vec.h:286:74: warning: ignoring attributes on template argument ‘VecType<float>::type’ {aka ‘__m512’} [-Wignored-attributes] 286 | using type = typename std::array<typename VecType<float>::type, num_vec>; |

Thanks. I guess we should fix most of these warnings instead of just ignoring them. @xuhancn can you take a look?

xuhancn · 2024-04-23T01:50:29Z

@jingxu10 will help on land this PR.

jingxu10 · 2024-04-23T02:09:55Z

Hi @AngryLoki there are several flake8 format issues. Could you follow the steps below to run the flake8 check at your side?

python -m pip install -r scripts/tools/setup/requirements-flake8.txt
python scripts/tools/setup/flake8.py

AngryLoki · 2024-05-05T15:57:38Z

@jingxu10 Python 3.11.8 and flake8==3.8.2 (current version) has no complains

Details

python scripts/tools/setup/flake8.py
All done! ✨ 🍰 ✨
1 file left unchanged.

status code:  0
All done! ✨ 🍰 ✨
156 files left unchanged.

status code:  0
All done! ✨ 🍰 ✨
3 files left unchanged.

status code:  0
All done! ✨ 🍰 ✨
103 files left unchanged.

status code:  0
Pass!

Moreover, I don't change Python files in this this pull-request, so it should not break anything.

jingxu10 · 2024-05-31T02:40:50Z

We have a clangformat check which failed in our internal CI system.
We are currently working on establishing the format check in the external CI system. I'll update you once it is online.

xuhancn · 2024-06-12T18:27:22Z

FYI: https://github.com/intel/intel-extension-for-pytorch/blob/main/CONTRIBUTING.md#code-style @AngryLoki

AngryLoki · 2024-06-13T13:18:07Z

@xuhancn , thx, I've applied script from #654 and clang-format-diff.py fixed few line wraps and indentations. I also rebased over main branch just in case.

jingxu10 · 2024-07-03T00:27:26Z

Hi, we have enabled a CI to run format verification. When you rebase your pr, the CI task should be triggered automatically.

jingxu10 · 2024-07-17T09:31:31Z

Compilation failed.
build_ipex_err.log

AngryLoki · 2024-07-19T16:51:04Z

@jingxu10 , thank you for checking!
Yes, operator""f16 was added in gcc 13.1. And out of all options, only _mm512_set1_ph(static_cast<_Float16>(1.0f)) works in gcc 12 and produces no warnings:

#include <immintrin.h>

// note: clang 13 and gcc 11.4 don't support -mavx512fp16 at all

// works perfectly, but only since gcc 13.1 and clang 14.
// gcc 12.4 error: unable to find numeric literal operator 'operator""f16'
auto vec_zero1 = _mm512_set1_ph(1.0f16);

// gcc >= 13.1 warning: converting to '_Float16' from 'float' with greater conversion rank
// -Wno-narrowing removes warning, but it is not the best idea 
auto vec_zero2 = _mm512_set1_ph(1.0f);

// same as above: produces warning with gcc
auto vec_zero3 = _mm512_set1_ph((_Float16)1.0f);

// works everywhere, no warnings!
// (clang 13 and gcc 11.4 don't support -mavx512fp16)
auto vec_zero4 = _mm512_set1_ph(static_cast<_Float16>(1.0f));

(same in godbolt conformance view: https://godbolt.org/z/MdGbT7beT)

I've updated my pull-request with static_cast and rebased just in case.

AngryLoki · 2024-07-19T17:08:52Z

@DiweiSun , CI fails with

pip install lintrunner pip install lintrunner-adapters
ERROR: No matching distribution found for install

Could you update it? Thanks

-pip install lintrunner pip install lintrunner-adapters
+pip install lintrunner lintrunner-adapters

https://github.com/intel/intel-extension-for-pytorch/blob/5cc852eb850f8b42ac2d0fcca5709e6076e8042d/.github/workflows/format-check.yml#L32C11-L32C65

Signed-off-by: Sv. Lockal <[email protected]>

DiweiSun · 2024-09-05T03:35:57Z

@DiweiSun , CI fails with
pip install lintrunner pip install lintrunner-adapters
ERROR: No matching distribution found for install
Could you update it? Thanks
-pip install lintrunner pip install lintrunner-adapters
+pip install lintrunner lintrunner-adapters
https://github.com/intel/intel-extension-for-pytorch/blob/5cc852eb850f8b42ac2d0fcca5709e6076e8042d/.github/workflows/format-check.yml#L32C11-L32C65

Fixed with d12ebc5

jingxu10 · 2024-09-10T20:17:45Z

merged to main branch

AngryLoki mentioned this pull request Apr 9, 2024

Difficult CMake build system producing unexpected results with MKL >= 2021.3.0 #528

Closed

jgong5 approved these changes Apr 9, 2024

View reviewed changes

AngryLoki force-pushed the fix-compile branch from 8039b65 to 3ac17b1 Compare June 13, 2024 13:13

AngryLoki force-pushed the fix-compile branch from 3ac17b1 to 5c7cae1 Compare July 3, 2024 05:52

AngryLoki force-pushed the fix-compile branch from 5c7cae1 to 5028b3f Compare July 19, 2024 16:46

Fix compilation with GCC and Clang

f70a470

Signed-off-by: Sv. Lockal <[email protected]>

AngryLoki force-pushed the fix-compile branch from 5028b3f to f70a470 Compare August 14, 2024 09:02

jingxu10 closed this Sep 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix compilation with GCC and Clang #587

Fix compilation with GCC and Clang #587

AngryLoki commented Apr 9, 2024 •

edited

Loading

jgong5 left a comment

jgong5 Apr 9, 2024

AngryLoki Apr 10, 2024

jgong5 Apr 11, 2024

xuhancn commented Apr 23, 2024

jingxu10 commented Apr 23, 2024

AngryLoki commented May 5, 2024

jingxu10 commented May 31, 2024

xuhancn commented Jun 12, 2024

AngryLoki commented Jun 13, 2024

jingxu10 commented Jul 3, 2024

jingxu10 commented Jul 17, 2024

AngryLoki commented Jul 19, 2024 •

edited

Loading

AngryLoki commented Jul 19, 2024

DiweiSun commented Sep 5, 2024

jingxu10 commented Sep 10, 2024

Fix compilation with GCC and Clang #587

Fix compilation with GCC and Clang #587

Conversation

AngryLoki commented Apr 9, 2024 • edited Loading

jgong5 left a comment

Choose a reason for hiding this comment

jgong5 Apr 9, 2024

Choose a reason for hiding this comment

AngryLoki Apr 10, 2024

Choose a reason for hiding this comment

jgong5 Apr 11, 2024

Choose a reason for hiding this comment

xuhancn commented Apr 23, 2024

jingxu10 commented Apr 23, 2024

AngryLoki commented May 5, 2024

jingxu10 commented May 31, 2024

xuhancn commented Jun 12, 2024

AngryLoki commented Jun 13, 2024

jingxu10 commented Jul 3, 2024

jingxu10 commented Jul 17, 2024

AngryLoki commented Jul 19, 2024 • edited Loading

AngryLoki commented Jul 19, 2024

DiweiSun commented Sep 5, 2024

jingxu10 commented Sep 10, 2024

AngryLoki commented Apr 9, 2024 •

edited

Loading

AngryLoki commented Jul 19, 2024 •

edited

Loading