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

Fix + improve vprintf implementation #211

Merged
merged 3 commits into from
May 15, 2024
Merged

Fix + improve vprintf implementation #211

merged 3 commits into from
May 15, 2024

Conversation

NyanCatTW1
Copy link
Contributor

@NyanCatTW1 NyanCatTW1 commented Apr 23, 2024

We were trying to debug a kernel with printf, but then we realized %d and friends didn't work at all! So we dug in and made changes to vprintf that should allow it to support all sorts of common printf calls

Test code (mostly taken from https://cplusplus.com/reference/cstdio/printf/)

#include <stdio.h>
#include <cuda.h>

__global__ void kernel() {
    printf("Characters: %c %c \n", 'a', 65);
    printf("Decimals: %d %ld\n", 1977, 650000L);
    printf("Preceding with blanks: %10d \n", 1977);
    printf("Preceding with zeros: %010d \n", 1977);
    printf("Some different radices: %d %x %o %#x %#o \n", 100, 100, 100, 100, 100);
    printf("floats: %4.2f %+.0e %E \n", 3.1416, 3.1416, 3.1416);
    printf("Width trick: %*d \n", 5, 10);
    printf("%s \n", "A string");
    printf("I'm not formatting anything: %%s!\n");
}

Expected output (currently identical to CUDA's)

Characters: a A 
Decimals: 1977 650000
Preceding with blanks:       1977 
Preceding with zeros: 0000001977 
Some different radices: 100 64 144 0x64 0144 
floats: 3.14 +3e+00 3.141600E+00 
Width trick:    10 
A string 
I'm not formatting anything: %s!

Some of the code could've been done better, but well, at least now it works properly

Copy link
Owner

@vosen vosen left a comment

Choose a reason for hiding this comment

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

Mostly good, but I need some small changes

ptx/lib/zluda_ptx_impl.cpp Outdated Show resolved Hide resolved
ptx/lib/zluda_ptx_impl.cpp Show resolved Hide resolved
ptx/lib/zluda_ptx_impl.cpp Show resolved Hide resolved
@vosen
Copy link
Owner

vosen commented May 4, 2024

Some of the code could've been done better, but well, at least now it works properly

Yes, te vprintf code is not very good. And difficult to test properly: last time I tried to capture from stdout handle it didn't work with HIP.
I should probably split printf into a separate file and test it on CPU with mocked __ockl_printf_*, but it's a story for another day.

@NyanCatTW1 NyanCatTW1 requested a review from vosen May 8, 2024 16:03
@vosen vosen merged commit fcd7a57 into vosen:master May 15, 2024
2 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.

2 participants