-
Notifications
You must be signed in to change notification settings - Fork 160
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 64 bit integer types on 32 bit archs #186
base: develop
Are you sure you want to change the base?
Conversation
In case the code is built with C89 on Linux LP32, 64 bit integers should be defined as long long types. limits.h can be used to differentiate between LP64 and LP32.
Thanks for bringing up this issue--we should be more careful about how we detect data models. I am a little puzzled about your solution, however. In the LP32 data model, ints are 16 bits and longs are 32 bits, so We're hoping to one day switch over to C99 stdint types, with custom C89 headers that provide corresponding types. This would similarly require carefully written code that detects the data model. If there's publicly available code that achieves this in a portable way, we'd certainly be interested. |
Actually, this should be ILP32 here. LP32 would be e.g. Win16, but I don't think this is relevant anymore. |
I think we need to do a better job detecting the data model, and in a portable manner. I've begun some work on this and would appreciate some feedback. I've attached a sample piece of code as a first step. @StefanBruens We don't have access to an ILP32 system, so could you please compile and run this code and let us know whether it does the right thing for you?
|
You can compile and run it on any x86_64 system (provided you have 32 bit libraries installed), e.g.: This reports: This is obviously incorrect. The reason is simple - while ILP32 may or may not have a 64 bit integer type (typically it does), the distinction between ILP32 an LLP64 is not the 64bit integer, but |
Good point, but I don't know of anything in zfp that depends on the storage of pointers, i.e., |
I don't say you need it, I only wanted to point out the posted code is wrong. It does not detect LLP64 vs ILP32 correctly. But when you look at my MR, it has |
I see. Well, I suppose it would be nice to correctly detect pointer size, but I cannot think of how to do that in a portable way as |
In case the code is built with C89 on Linux LP32, 64 bit integers should be defined as long long types. limits.h can be used to differentiate between LP64 and LP32.