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

use ssize_t/int64 and 64bit version of ftell on windows #179

Merged
merged 3 commits into from
Aug 1, 2023

Conversation

richinseattle
Copy link
Contributor

On Windows, there is a separate API for ftell on 64bit, in addition due to long type being 32bit on Windows, switch to ssize_t

@karpathy
Copy link
Owner

Sorry can you say a bit more? Does the current code not work/run properly?

@richinseattle
Copy link
Contributor Author

This was a fix for #165 This patch will successfully load llama2-7b after converting it.
On Windows the long type is 32bit and the ftell() api is only 32bit, limiting use to 2GB files.

On Windows, the distinct api for 64bit ftell is ftelli64() so that was a simple update. But then we need to use a type that is appropriate for 64bit sizes with possible -1 return value, so ssize_t makes sense. I updated the mmap code to use 64bit sizes which simplified some of the logic, but technically it will still work on 32bit as well my commit comment was a little misleading.

@karpathy karpathy merged commit b7f026f into karpathy:master Aug 1, 2023
vinhtran2611 pushed a commit to vinhtran2611/llama2.c that referenced this pull request Jan 20, 2024
use ssize_t/int64 and 64bit version of ftell on windows
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