Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SIMD is wrongly enabled while building on riscv64gc #109

Closed
Cryolitia opened this issue Oct 31, 2024 · 7 comments · Fixed by #110
Closed

SIMD is wrongly enabled while building on riscv64gc #109

Cryolitia opened this issue Oct 31, 2024 · 7 comments · Fixed by #110

Comments

@Cryolitia
Copy link
Contributor

-- ------------------------------------------------------------------------------
--     libunicode (version 0.6.0)
-- ------------------------------------------------------------------------------
-- Build type:                  None
-- Build mode:                  dynamic
-- Build unit tests:            OFF
-- Build benchmark:             OFF
-- Build tools:                 ON
-- Enable tablegen fast build:  OFF
-- Using ccache:                CCACHE-NOTFOUND
-- SIMD support:                intrinsics
-- Using UCD directory:         /build/libunicode/src/libunicode/_ucd/ucd-16.0.0
-- Enable clang-tidy:           OFF ()
-- ------------------------------------------------------------------------------
-- ==============================================================================
--     ThirdParties
-- ------------------------------------------------------------------------------
-- ------------------------------------------------------------------------------
-- Configuring done (44.2s)
-- Generating done (0.2s)
-- Build files have been written to: /build/libunicode/src/build
[0/24] Generating UCD API and tables from /build/libunicode/src/libunicode/_ucd/ucd-16.0.0
[2/24] Building CXX object src/libunicode/CMakeFiles/unicode_ucd.dir/ucd.cpp.o
[3/24] Building CXX object src/libunicode/CMakeFiles/unicode_tablegen.dir/tablegen.cpp.o
[4/24] Linking CXX shared library src/libunicode/libunicode_ucd.so.0.6.0
[5/24] Creating library symlink src/libunicode/libunicode_ucd.so.0.6 src/libunicode/libunicode_ucd.so
[6/24] Building CXX object src/libunicode/CMakeFiles/unicode_loader.dir/codepoint_properties_loader.cpp.o
[7/24] Linking CXX shared library src/libunicode/libunicode_loader.so.0.6.0
[8/24] Creating library symlink src/libunicode/libunicode_loader.so.0.6 src/libunicode/libunicode_loader.so
[9/24] Linking CXX executable src/libunicode/unicode_tablegen
[9/24] Generating UCD codepoint properties tables from /build/libunicode/src/libunicode/_ucd/ucd-16.0.0
Loading file Scripts.txt ... 152 ms
Loading file DerivedCoreProperties.txt ... 321 ms
Loading file DerivedAge.txt ... 135 ms
Loading file extracted/DerivedGeneralCategory.txt ... 399 ms
Loading file extracted/DerivedName.txt ... 376 ms
Loading file auxiliary/GraphemeBreakProperty.txt ... 28 ms
Loading file EastAsianWidth.txt ... 124 ms
Loading file emoji/emoji-data.txt ... 27 ms
Assigning EmojiSegmentationCategory ... 16 ms
Assigning char_width ... 6 ms
Creating multistage tables (properties) ... 24919 ms
Creating multistage tables (names) ... 640469 ms
Writing C++ table files ... 215 ms
[11/24] Building CXX object src/libunicode/CMakeFiles/unicode.dir/utf8.cpp.o
[12/24] Building CXX object src/libunicode/CMakeFiles/unicode.dir/script_segmenter.cpp.o
[13/24] Building CXX object src/libunicode/CMakeFiles/unicode.dir/width.cpp.o
[14/24] Building CXX object src/libunicode/CMakeFiles/unicode.dir/codepoint_properties.cpp.o
[15/24] Building CXX object src/libunicode/CMakeFiles/unicode.dir/scan.cpp.o
FAILED: src/libunicode/CMakeFiles/unicode.dir/scan.cpp.o 
/usr/bin/c++ -DUSE_INTRINSICS -D_GLIBCXX_DEBUG -Dunicode_EXPORTS -I/build/libunicode/src/libunicode/src -I/build/libunicode/src/libunicode/src/libunicode/.. -march=rv64gc -mabi=lp64d -O2 -pipe -fno-plt -fexceptions         -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security         -fstack-clash-protection         -fno-omit-frame-pointer -Wp,-D_GLIBCXX_ASSERTIONS -g -ffile-prefix-map=/build/libunicode/src=/usr/src/debug/libunicode -flto=auto -std=c++20 -fPIC -fdiagnostics-color=always -Wall -Wextra -fdiagnostics-color=always -Wconversion -Wduplicated-cond -Wextra-semi -Wimplicit-fallthrough -Wlogical-op -Wmissing-declarations -Wno-unknown-pragmas -Wnull-dereference -Wpessimizing-move -Wredundant-move -Wsign-conversion -pedantic -MD -MT src/libunicode/CMakeFiles/unicode.dir/scan.cpp.o -MF src/libunicode/CMakeFiles/unicode.dir/scan.cpp.o.d -o src/libunicode/CMakeFiles/unicode.dir/scan.cpp.o -c /build/libunicode/src/libunicode/src/libunicode/scan.cpp
/build/libunicode/src/libunicode/src/libunicode/scan.cpp: In function ‘size_t unicode::detail::scan_for_text_ascii(std::string_view, size_t)’:
/build/libunicode/src/libunicode/src/libunicode/scan.cpp:110:5: error: ‘intrinsics’ has not been declared
  110 |     intrinsics::m128i const ControlCodeMax = intrinsics::set1_epi8(0x20); // 0..0x1F
      |     ^~~~~~~~~~
/build/libunicode/src/libunicode/src/libunicode/scan.cpp:111:5: error: ‘intrinsics’ has not been declared
  111 |     intrinsics::m128i const Complex = intrinsics::set1_epi8(-128);        // equals to 0x80 (0b1000'0000)
      |     ^~~~~~~~~~
/build/libunicode/src/libunicode/src/libunicode/scan.cpp:113:33: error: ‘intrinsics’ has not been declared
  113 |     while (input < end - sizeof(intrinsics::m128i))
      |                                 ^~~~~~~~~~
/build/libunicode/src/libunicode/src/libunicode/scan.cpp:115:9: error: ‘intrinsics’ has not been declared
  115 |         intrinsics::m128i batch = intrinsics::load_unaligned((intrinsics::m128i*) input);
      |         ^~~~~~~~~~
/build/libunicode/src/libunicode/src/libunicode/scan.cpp:116:9: error: ‘intrinsics’ has not been declared
  116 |         intrinsics::m128i isControl = intrinsics::compare_less(batch, ControlCodeMax);
      |         ^~~~~~~~~~
/build/libunicode/src/libunicode/src/libunicode/scan.cpp:117:9: error: ‘intrinsics’ has not been declared
  117 |         intrinsics::m128i isComplex = intrinsics::and128(batch, Complex);
      |         ^~~~~~~~~~
/build/libunicode/src/libunicode/src/libunicode/scan.cpp:119:9: error: ‘intrinsics’ has not been declared
  119 |         intrinsics::m128i testPack = intrinsics::or128(isControl, isComplex);
      |         ^~~~~~~~~~
/build/libunicode/src/libunicode/src/libunicode/scan.cpp:120:31: error: ‘intrinsics’ has not been declared
  120 |         if (int const check = intrinsics::movemask_epi8(testPack); check != 0)
      |                               ^~~~~~~~~~
/build/libunicode/src/libunicode/src/libunicode/scan.cpp:120:57: error: ‘testPack’ was not declared in this scope
  120 |         if (int const check = intrinsics::movemask_epi8(testPack); check != 0)
      |                                                         ^~~~~~~~
/build/libunicode/src/libunicode/src/libunicode/scan.cpp:126:25: error: ‘intrinsics’ has not been declared
  126 |         input += sizeof(intrinsics::m128i);
      |                         ^~~~~~~~~~

Link: https://archriscv.felixc.at/.status/log.htm?url=logs/libunicode/libunicode-0.6.0-2.log

@Yaraslaut
Copy link
Member

Hi, unfortunately cmake does not provide built-in way to check arhitecture, and at the moment you need to disable explicitly simd usage in libunicode with cmake flag -DLIBUNICODE_SIMD_IMPLEMENTATION=none
In the future we want to add vector extenstion riscv usage, but for now we do not support simd for riscv

@Cryolitia
Copy link
Contributor Author

Cryolitia commented Oct 31, 2024

Thanks so much for your response!

I would like to know one more not too related thing. If I compile it with SIMD enabled, what would happened while running it on a x86_64-v1 hardware. It's important for us distros maintainers, because most distros' package baseline is x86_64-v1, we would like to promise its availability on it, as a result of that SIMD is enabled by default that some distros may not notice it.


update:

cmake does not provide built-in way to check arhitecture

Could we use https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_PROCESSOR.html

Cryolitia added a commit to Cryolitia-Forks/archriscv-packages that referenced this issue Nov 1, 2024
@Yaraslaut
Copy link
Member

Yes, we can try to check if during cmake configuration.
As for the SIMD support, at the moment intrinsics are using SSE2, so x86_64-v1, in the future this usage will be preserved and ideally checked at run-time which SIMD implementation we can use (#108)

felixonmars pushed a commit to felixonmars/archriscv-packages that referenced this issue Nov 3, 2024
@christianparpart
Copy link
Member

@Yaraslaut we should be able to runtime detect architecture by using uname command line tool on UNIX like systems.

Otherwise, shouldn't something like this work as well?

if(CMAKE_SYSTEM_PROCESSOR STREQUAL "riscv64gc")
    set(LIBUNICODE_SIMD_IMPLEMENTATION none) # currently unsupported on risckv64gc
endif()

@Cryolitia
Copy link
Contributor Author

if(CMAKE_SYSTEM_PROCESSOR STREQUAL "riscv64gc")

s/riscv64gc/riscv64/g :)

@christianparpart
Copy link
Member

if(CMAKE_SYSTEM_PROCESSOR STREQUAL "riscv64gc")

s/riscv64gc/riscv64/g :)

I just copy'n'pasted the word from the title. I don't have any RISCV experience nor access. If that works, we can gladly add it to the CMakeLists.txt. Could you confirm this then?

@Cryolitia
Copy link
Contributor Author

Sorry for lont time AFK.

I have create a PR: #110

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants