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

clang-tidy #270

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

clang-tidy #270

wants to merge 1 commit into from

Conversation

ngc92
Copy link
Contributor

@ngc92 ngc92 commented Apr 27, 2024

Adds a clang-tidy file and clang-tidy target to the make file.
Since the .cu files are in flux right now, this is just looking at gpt2.c

I'm not quite sure which checks we should enable, but I think the ones that I've selected should be a good starting point.
In particular, there is one warning about using the result of 32-bit matrix multiplications as an offset in a pointer operation -- that seems like a good one to have, to prevent stuff like the high batch-size crashes.

Regarding the "fixes" I've made to ensure we don't get these warnings, I'm not 100% sure about those, and even more so about using ptrdiff_t vs size_t.

(using size_t for the local B, T, etc variables gets us 64-bit offset calculations, but also implementation-defined narrowing once we call the actual kernels)

@dagelf
Copy link
Contributor

dagelf commented Apr 30, 2024

Wow, this is neat. Might as well add all the *.c* files!

Is there any easy way to ignore just the printfs return values not used? 😄

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.

None yet

2 participants