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

Address short comings with printf for strings #715

merged 29 commits into from
Nov 6, 2024

Conversation

Rekt3421
Copy link
Contributor

This change fixes short comings encountered in 9 tests of the OpenCL-CTS as mentioned in issue #714 we still need to fix kernel descriptors.

@Rekt3421
Copy link
Contributor Author

Next step is to add some unit tests

src/printf.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@rjodinchr rjodinchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure you only modify what really needs to change.

src/printf.cpp Outdated Show resolved Hide resolved
src/printf.cpp Outdated Show resolved Hide resolved
tests/api/printf.cpp Outdated Show resolved Hide resolved
@rjodinchr
Copy link
Contributor

This is fixing CTS printf string, but printf format_string is regressing (segfault with "\"%%\"").

We should maybe try to address everything (string & format_string) together.

@Rekt3421
Copy link
Contributor Author

current status 3 sub-subtests are still failing. One which is due to some issue with printf itself.

I.e. in strings subtest there is a subtest

printf(%%foo%%) 

is printing out

%%foo%

which is a bit confusing since the two %% should be consumed as one.

also in the same subtests for

   printf("%s\n","foo\0foo");                                                           

the kernel descriptors are not properly populated for some reason.

a vector sub-subtest is also failing
for which I do not have an explanation yet.

src/printf.cpp Outdated Show resolved Hide resolved
src/printf.cpp Outdated Show resolved Hide resolved
src/printf.cpp Outdated Show resolved Hide resolved
src/printf.cpp Outdated Show resolved Hide resolved
src/printf.cpp Outdated Show resolved Hide resolved
src/printf.cpp Outdated Show resolved Hide resolved
@Rekt3421
Copy link
Contributor Author

Rekt3421 commented Oct 7, 2024

@kpet to get this PR working correctly we need to update clspv with the fixes landed by David

src/printf.cpp Outdated Show resolved Hide resolved
@kpet kpet merged commit d944acd into kpet:main Nov 6, 2024
9 checks passed
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 this pull request may close these issues.

3 participants