This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Enable SLP by default (when vectors are available)
ClosedPublic

Authored by reames on Jun 12 2023, 1:45 PM.

Details

Summary

I propose that we go ahead and enabled SLP by default. Over the last few weeks, @luke and I have been working through codegen issues seen at small VLs from a couple of SPEC workloads. We still have a ways to go to get optimal codegen, but we're at the point where having a single configuration we're all tuning against is probably the right default.

As a bit of history, I introduced this TTI hook back in a310637132 back in August of last year to unblock enabling LoopVectorizer. At the time, we had a couple known issues: constant materialization, address generation, and a general lack of maturity of small fixed vector codegen. By now, each of these has had significant investment. I can't say any of them are completely fixed, but we're no longer seeing instances of them every place we look.

What we're mostly seeing at this point is a long tail of code gen opportunities, many involving build vectors, shuffles, and extract patterns. I have a couple patches up to continue iterating on those issues, but I don't think they need to be blockers for enabling SLP. The only debateable one is D152001 , and I'm prepared to leave it up to which gets approved first. :)

Diff Detail

Event Timeline

reames created this revision.Jun 12 2023, 1:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2023, 1:45 PM
reames requested review of this revision.Jun 12 2023, 1:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2023, 1:45 PM
ABataev accepted this revision.Jun 12 2023, 1:51 PM

I can try to provide a better fix for D152001 after vacation (2 weeks)

This revision is now accepted and ready to land.Jun 12 2023, 1:51 PM
This revision was landed with ongoing or failed builds.Jun 14 2023, 9:58 AM
This revision was automatically updated to reflect the committed changes.

Hi. It looks like this is failing to build for us on windows host https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8778294144728066337/+/u/clang/build/stdout?format=raw with:

FAILED: lib/Target/RISCV/CMakeFiles/LLVMRISCVCodeGen.dir/RISCVTargetTransformInfo.cpp.obj 
C:\b\s\w\ir\x\w\cipd\bin\clang-cl.exe  /nologo -TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_LIBCPP_ENABLE_ASSERTIONS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\b\s\w\ir\x\w\staging\llvm_build\lib\Target\RISCV -IC:\b\s\w\ir\x\w\llvm-llvm-project\llvm\lib\Target\RISCV -IC:\b\s\w\ir\x\w\recipe_cleanup\tensorflow-venv\store\python_venv-b2t7sq1hdmljg9g7sc6p05hel8\contents\Lib\site-packages\tensorflow\include -IC:\b\s\w\ir\x\w\staging\llvm_build\include -IC:\b\s\w\ir\x\w\llvm-llvm-project\llvm\include -imsvcC:\b\s\w\ir\x\w\staging\zlib_install_target\include -imsvcC:\b\s\w\ir\x\w\staging\zstd_install\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:__cplusplus /Oi /Brepro /bigobj /permissive- /W4  -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported /Gw -no-canonical-prefixes /MT /O2 /Ob2 -std:c++17  /EHs-c- /GR- -UNDEBUG /showIncludes /Folib\Target\RISCV\CMakeFiles\LLVMRISCVCodeGen.dir\RISCVTargetTransformInfo.cpp.obj /Fdlib\Target\RISCV\CMakeFiles\LLVMRISCVCodeGen.dir\LLVMRISCVCodeGen.pdb -c -- C:\b\s\w\ir\x\w\llvm-llvm-project\llvm\lib\Target\RISCV\RISCVTargetTransformInfo.cpp
C:\b\s\w\ir\x\w\llvm-llvm-project\llvm\lib\Target\RISCV\RISCVTargetTransformInfo.cpp(1759,10): error: no matching function for call to 'max'
 1759 |   return std::max(1UL, RegWidth.getFixedValue() / ElemWidth);
      |          ^~~~~~~~
C:\b\s\w\ir\cache\windows_sdk\VC\Tools\MSVC\14.34.31933\include\utility(41,6): note: candidate template ignored: deduced conflicting types for parameter '_Ty' ('unsigned long' vs. 'ScalarTy' (aka 'unsigned long long'))
   41 |     (max) (const _Ty& _Left, const _Ty& _Right) noexcept(noexcept(_Left < _Right)) /* strengthened */ {
      |      ^
C:\b\s\w\ir\cache\windows_sdk\VC\Tools\MSVC\14.34.31933\include\algorithm(9868,26): note: candidate template ignored: could not match 'initializer_list<_Ty>' against 'unsigned long'
 9868 | _NODISCARD constexpr _Ty(max)(initializer_list<_Ty> _Ilist, _Pr _Pred) {
      |                          ^
C:\b\s\w\ir\cache\windows_sdk\VC\Tools\MSVC\14.34.31933\include\algorithm(9875,26): note: candidate function template not viable: requires single argument '_Ilist', but 2 arguments were provided
 9875 | _NODISCARD constexpr _Ty(max)(initializer_list<_Ty> _Ilist) {
      |                          ^
C:\b\s\w\ir\cache\windows_sdk\VC\Tools\MSVC\14.34.31933\include\utility(31,33): note: candidate function template not viable: requires 3 arguments, but 2 were provided
   31 | _NODISCARD constexpr const _Ty&(max) (const _Ty& _Left, const _Ty& _Right, _Pr _Pred) noexcept(
      |                                 ^
1 error generated.

Would you be able to send out a fix or revert? Thanks.

Hi. It looks like this is failing to build for us on windows host https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8778294144728066337/+/u/clang/build/stdout?format=raw with:

FAILED: lib/Target/RISCV/CMakeFiles/LLVMRISCVCodeGen.dir/RISCVTargetTransformInfo.cpp.obj 
C:\b\s\w\ir\x\w\cipd\bin\clang-cl.exe  /nologo -TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_LIBCPP_ENABLE_ASSERTIONS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\b\s\w\ir\x\w\staging\llvm_build\lib\Target\RISCV -IC:\b\s\w\ir\x\w\llvm-llvm-project\llvm\lib\Target\RISCV -IC:\b\s\w\ir\x\w\recipe_cleanup\tensorflow-venv\store\python_venv-b2t7sq1hdmljg9g7sc6p05hel8\contents\Lib\site-packages\tensorflow\include -IC:\b\s\w\ir\x\w\staging\llvm_build\include -IC:\b\s\w\ir\x\w\llvm-llvm-project\llvm\include -imsvcC:\b\s\w\ir\x\w\staging\zlib_install_target\include -imsvcC:\b\s\w\ir\x\w\staging\zstd_install\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:__cplusplus /Oi /Brepro /bigobj /permissive- /W4  -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported /Gw -no-canonical-prefixes /MT /O2 /Ob2 -std:c++17  /EHs-c- /GR- -UNDEBUG /showIncludes /Folib\Target\RISCV\CMakeFiles\LLVMRISCVCodeGen.dir\RISCVTargetTransformInfo.cpp.obj /Fdlib\Target\RISCV\CMakeFiles\LLVMRISCVCodeGen.dir\LLVMRISCVCodeGen.pdb -c -- C:\b\s\w\ir\x\w\llvm-llvm-project\llvm\lib\Target\RISCV\RISCVTargetTransformInfo.cpp
C:\b\s\w\ir\x\w\llvm-llvm-project\llvm\lib\Target\RISCV\RISCVTargetTransformInfo.cpp(1759,10): error: no matching function for call to 'max'
 1759 |   return std::max(1UL, RegWidth.getFixedValue() / ElemWidth);
      |          ^~~~~~~~
C:\b\s\w\ir\cache\windows_sdk\VC\Tools\MSVC\14.34.31933\include\utility(41,6): note: candidate template ignored: deduced conflicting types for parameter '_Ty' ('unsigned long' vs. 'ScalarTy' (aka 'unsigned long long'))
   41 |     (max) (const _Ty& _Left, const _Ty& _Right) noexcept(noexcept(_Left < _Right)) /* strengthened */ {
      |      ^
C:\b\s\w\ir\cache\windows_sdk\VC\Tools\MSVC\14.34.31933\include\algorithm(9868,26): note: candidate template ignored: could not match 'initializer_list<_Ty>' against 'unsigned long'
 9868 | _NODISCARD constexpr _Ty(max)(initializer_list<_Ty> _Ilist, _Pr _Pred) {
      |                          ^
C:\b\s\w\ir\cache\windows_sdk\VC\Tools\MSVC\14.34.31933\include\algorithm(9875,26): note: candidate function template not viable: requires single argument '_Ilist', but 2 arguments were provided
 9875 | _NODISCARD constexpr _Ty(max)(initializer_list<_Ty> _Ilist) {
      |                          ^
C:\b\s\w\ir\cache\windows_sdk\VC\Tools\MSVC\14.34.31933\include\utility(31,33): note: candidate function template not viable: requires 3 arguments, but 2 were provided
   31 | _NODISCARD constexpr const _Ty&(max) (const _Ty& _Left, const _Ty& _Right, _Pr _Pred) noexcept(
      |                                 ^
1 error generated.

Would you be able to send out a fix or revert? Thanks.

defb5cd783e1