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

Address short comings with printf for strings #715

Merged
merged 29 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9a2b9e2
keep this
Rekt3421 Aug 14, 2024
8046dc2
rework how process_printf handles strings
Rekt3421 Aug 14, 2024
4a33381
add missing space
Rekt3421 Aug 14, 2024
45f2ae4
fix cases with no format
Rekt3421 Aug 27, 2024
a7af5cb
use assert instead of if and return earlier.
Rekt3421 Aug 28, 2024
8c606c7
move data to end instead of asserting
Rekt3421 Aug 28, 2024
c7d90c4
Merge branch 'main' into 714
Rekt3421 Aug 28, 2024
1cd3365
reinstate comment format
Rekt3421 Aug 29, 2024
ff36633
reinstate comment format
Rekt3421 Aug 29, 2024
33e43e6
add unit test for original failure
Rekt3421 Sep 9, 2024
f1d81ce
Merge branch 'main' into 714
Rekt3421 Sep 9, 2024
ebec0c4
rework test logic
Rekt3421 Sep 9, 2024
467ffad
use correct class
Rekt3421 Sep 9, 2024
cbd9ff3
rework logic
Rekt3421 Sep 9, 2024
d0a44b6
get rid of sprintf
Rekt3421 Sep 11, 2024
7cca2d8
Fix issues with format_string
Rekt3421 Sep 24, 2024
f688a80
handle quotes
Rekt3421 Sep 25, 2024
9557840
fix vector of len>10 and %% cases
Rekt3421 Sep 26, 2024
2380ff8
make less complex
Rekt3421 Oct 1, 2024
852e14c
condense logic further
Rekt3421 Oct 2, 2024
49fc753
revert untouched code
Rekt3421 Oct 2, 2024
181a82c
commit romaric's suggestion
Rekt3421 Oct 2, 2024
41c9d79
remove uneeded checks
Rekt3421 Oct 3, 2024
39dda98
restore the condition
Rekt3421 Oct 3, 2024
9c25fbe
get rid of condition again
Rekt3421 Oct 7, 2024
6d85e0c
Add romaric's suggestion back in
Rekt3421 Oct 8, 2024
e7936e4
add two tests for new functionality
Rekt3421 Oct 16, 2024
c0a381c
add one last test
Rekt3421 Oct 16, 2024
b3a5159
fix test for while loop
Rekt3421 Oct 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions src/printf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ std::string get_vector_fmt(std::string fmt, int& vector_size, int& element_size,
// Consume precision and field width
pos = fmt.find_first_not_of("123456789.", pos);

if (fmt.at(pos) != 'v') {
if (pos == std::string::npos || fmt.at(pos) != 'v') {
vector_size = 1;
return std::string{fmt};
}
Expand Down Expand Up @@ -165,14 +165,20 @@ void process_printf(char*& data, const printf_descriptor_map_t& descs,

std::stringstream printf_out{};

// Firstly print the part of the format string up to the first '%'
// Firstly print the part of the format string up to the first '%' if any
// otherwise print the whole string as is and move the data pointer to the
// end.
size_t next_part = format_string.find_first_of('%');
if (next_part == std::string::npos) {
next_part = format_string.size();
data = data_end;
}
printf_out << format_string.substr(0, next_part);

// Decompose the remaining format string into individual strings with
// one format specifier each, handle each one individually
size_t arg_idx = 0;
while (next_part < format_string.size() - 1) {
while (next_part < format_string.size()) {
// Get the part of the format string before the next format specifier
size_t part_start = next_part;
size_t part_end = format_string.find_first_of('%', part_start + 1);
Expand All @@ -181,7 +187,15 @@ void process_printf(char*& data, const printf_descriptor_map_t& descs,
// Handle special cases
if (part_end == part_start + 1) {
printf_out << "%";
next_part = part_end + 1;
// We also need to print the literals between '%%' and the next '%'
next_part = part_start = part_end + 1;
part_end = format_string.find_first_of('%', part_start);
if (part_end != std::string::npos && part_end > part_start) {
part_fmt =
format_string.substr(part_start, part_end - part_start);
printf_out << part_fmt;
next_part = part_end;
}
continue;
} else if (part_end == std::string::npos &&
arg_idx >= descs.at(printf_id).arg_sizes.size()) {
Expand Down
61 changes: 61 additions & 0 deletions tests/api/printf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,23 @@ TEST_F(WithCommandQueueAndPrintf, SimplePrintf) {
char source[512];
sprintf(source, "kernel void test_printf() { printf(\"%s\");}", message);
auto kernel = CreateKernel(source, "test_printf");
size_t gws = 1;
size_t lws = 1;
EnqueueNDRangeKernel(kernel, 1, nullptr, &gws, &lws, 0, nullptr, nullptr);
Finish();
ASSERT_STREQ(m_printf_output.c_str(), message);
}

TEST_F(WithCommandQueueAndPrintf, SimplePrintfPercent) {
// Tests that the while loop inside the process_printf func
// goes all the way to the end of the string to be printed.
const char message[] = "The value is: 123%";
char source[512];
sprintf(source,
"kernel void test_printf() { printf(\"The value is: %d%%\"); }",
123);

auto kernel = CreateKernel(source, "test_printf");

size_t gws = 1;
size_t lws = 1;
Expand All @@ -32,6 +49,50 @@ TEST_F(WithCommandQueueAndPrintf, SimplePrintf) {
ASSERT_STREQ(m_printf_output.c_str(), message);
}

TEST_F(WithCommandQueueAndPrintf, SimplePrintfBetweenPercents) {
const char message[] = "%Hello%World%!";
const char message_test[] = "%%Hello%%World%%!";

char source[512];
sprintf(source, "kernel void test_printf() { printf(\"%s\");}",
message_test);

auto kernel = CreateKernel(source, "test_printf");

size_t gws = 1;
size_t lws = 1;
EnqueueNDRangeKernel(kernel, 1, nullptr, &gws, &lws, 0, nullptr, nullptr);
Finish();

ASSERT_STREQ(m_printf_output.c_str(), message);
}

TEST_F(WithCommandQueueAndPrintf, SimpleFormatedPrintf) {
const char* source = "kernel void test_printf() { printf(\"%s\", \"\"); }";
auto kernel = CreateKernel(source, "test_printf");

size_t gws = 1;
size_t lws = 1;
EnqueueNDRangeKernel(kernel, 1, nullptr, &gws, &lws, 0, nullptr, nullptr);
Finish();

ASSERT_STREQ(m_printf_output.c_str(), "");
}
TEST_F(WithCommandQueueAndPrintf, PrintfWithNoFormatSpecifier) {
const char* source =
"kernel void test_printf() { printf(\"\\n\", \"foo\");}";
auto kernel = CreateKernel(source, "test_printf");

size_t gws = 1;
size_t lws = 1;
EnqueueNDRangeKernel(kernel, 1, nullptr, &gws, &lws, 0, nullptr, nullptr);
Finish();

// The expected output is just a newline character since there's no
// format specifier to consume the "foo" argument.
ASSERT_STREQ(m_printf_output.c_str(), "\n");
}

TEST_F(WithCommandQueueAndPrintf, TooLongPrintf) {
// each print takes 12 bytes (4 for the printf_id, and 2*4 for the 2 integer
// to print) + 4 for the byte written counter
Expand Down