RVV: add cmake toolchain for use Spike #395
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lemire
reviewed
Apr 8, 2024
| const auto rhs = (b); \ | ||
| if (lhs != rhs) { \ | ||
| std::stringstream lhs_str; \ | ||
| lhs_str << lhs; \ |
There was a problem hiding this comment.
We are getting errors such as...
D:\a\simdutf\simdutf\tests\base64_tests.cpp(95,5): message : did you mean to call the c_str() method? [D:\a\simdutf\simdutf\build\tests\base64_tests.vcxproj]
D:\a\simdutf\simdutf\tests\..\tests/helpers/test.h(52,34): message : expanded from macro 'ASSERT_EQUAL' [D:\a\simdutf\simdutf\build\tests\base64_tests.vcxproj]
D:\a\simdutf\simdutf\tests\base64_tests.cpp(95,5): error : cannot pass non-trivial object of type '_Mystr' (aka 'basic_string<char, std::char_traits<char>, std::allocator<char>>') to variadic function; expected type from format string was 'char *' [-Wnon-pod-varargs] [D:\a\simdutf\simdutf\build\tests\base64_tests.vcxproj]
D:\a\simdutf\simdutf\tests\..\tests/helpers/test.h(53,34): message : expanded from macro 'ASSERT_EQUAL' [D:\a\simdutf\simdutf\build\tests\base64_tests.vcxproj]
D:\a\simdutf\simdutf\tests\base64_tests.cpp(95,5): message : did you mean to call the c_str() method? [D:\a\simdutf\simdutf\build\tests\base64_tests.vcxproj]
D:\a\simdutf\simdutf\tests\..\tests/helpers/test.h(53,34): message : expanded from macro 'ASSERT_EQUAL' [D:\a\simdutf\simdutf\build\tests\base64_tests.vcxproj]
lemire
reviewed
Apr 8, 2024
| lhs_str << lhs; \ | ||
| std::stringstream rhs_str; \ | ||
| rhs_str << rhs; \ | ||
| printf("lhs: %s = %s\n", #a, lhs_str.str()); \ |
There was a problem hiding this comment.
Suggested change
| printf("lhs: %s = %s\n", #a, lhs_str.str()); \ | |
| printf("lhs: %s = %s\n", #a, lhs_str.str().c_str()); \ |
lemire
reviewed
Apr 8, 2024
| std::stringstream rhs_str; \ | ||
| rhs_str << rhs; \ | ||
| printf("lhs: %s = %s\n", #a, lhs_str.str()); \ | ||
| printf("rhs: %s = %s\n", #b, rhs_str.str()); \ |
There was a problem hiding this comment.
Suggested change
| printf("rhs: %s = %s\n", #b, rhs_str.str()); \ | |
| printf("rhs: %s = %s\n", #b, rhs_str.str().c_str()); \ |
|
Note that in modern C++ (C++20), we would just use Or, we could use |
Use of std::cout or std:cerr makes impossible to run programs under Spike & proxy kernel (`pk`). The initialization of these standard streams depends on futexes (in glibc) and `pk` does not implement all system calls. A side effect is being ready for use the `fmt` library.
WojciechMula
force-pushed
the
rvv-spike-support
branch
from
a7144ee
to
88cffcb
Compare
lemire
reviewed
Apr 9, 2024
| std::cout << " implementation.convert_utf16_to_utf32.second return " << utf16_to_utf32.second << std::endl; | ||
| printf("%s\n", (is_ok_utf16.second ? "UTF-16 is ok" : "UTF-16 is not ok")); | ||
| printf(" size = %llu\n", input.size() / sizeof(char16_t)); | ||
| printf(" implementation.convert_utf16_to_utf8.second return %llu\n", utf16_to_utf8.second); |
There was a problem hiding this comment.
The recommended expression to print a size_t value is %zu and not %llu.
lemire
reviewed
Apr 9, 2024
| @@ -374,8 +373,7 @@ TEST(roundtrip_base64) { | |||
| printf("=====input size %zu\n", len); | |||
| for (size_t i = 0; i < len; i++) { | |||
| if (back[i] != source[i]) { | |||
| std::cerr << "Mismatch at position " << i << " trial " << trial | |||
| << std::endl; | |||
| printf("Mismatch at position %llu trial %llu\n", i, trial); | |||
There was a problem hiding this comment.
Same comment here, should be %zu and not %llu.
|
@WojciechMula Your changes have triggered another bug in our test framework, see #404 Eventually, you should rebase on top of #404 Sorry. |
|
@WojciechMula Can you rebase? I think I fixed the issue your work outlined. |
|
Running tests. |
|
I am going to merge and then fix the issues. |
|
I have resolved the issue directly in the main branch. |
|
Thanks a lot! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current state of tests: