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 int32_t type instead of int #961

Merged
merged 12 commits into from
Jan 9, 2025
Merged

Use int32_t type instead of int #961

merged 12 commits into from
Jan 9, 2025

Conversation

foxtran
Copy link
Contributor

@foxtran foxtran commented Jan 2, 2025

This patch updates C API (and others too) for using int32_t type. After the patch, tracy should be more portable for cases if int type has a size different from 4 bytes.

Fixes #953

@wolfpld
Copy link
Owner

wolfpld commented Jan 2, 2025

Why does this matter? I don't think there is an expectation for int to be 32 bit in the cases you have changed (I have not looked thoroughly though).

Also, why have you added explicit conversion between bool and int?

@foxtran
Copy link
Contributor Author

foxtran commented Jan 2, 2025

Why does this matter?

I wrote bindings for Fortran. It is useful to have consistent types both in Fortran and C sides (except unsigned integers, Fortran does not have them). So for int in Fortran we have c_int type, for int32_t we have c_int32_t and so on. Unfortunately, one of compilers have its own understanding what is the size of int type in C (it can be changed between 32 or 64 bits by compiler flag), while normally it is 32 bit for GCC/LLVM. So, compiling of Fortran-app+Tracy using this compiler will not produce a binary which will work correctly if int type is 64 bit long in Fortran side and Tracy was compiled with GCC/LLVM.
So, I have changed C API in a way to have the precise size of int in all public functions and in corresponding places. It should not affect existing codes since for most cases int has 32 bits.

I don't think there is an expectation for int to be 32 bit in the cases you have changed (I have not looked thoroughly though).

Most of changes were related to depth which can be non-negative and in some places int is used as bool.
Anyway, it should be relatively simple to choose which types do we want to see.

Also, why have you added explicit conversion between bool and int?

Just for explicit conversion between types. Always nice to have :)

@foxtran
Copy link
Contributor Author

foxtran commented Jan 2, 2025

Ooops. I forgot about Python API.

@wolfpld
Copy link
Owner

wolfpld commented Jan 2, 2025

Unfortunately, one of compilers have its own understanding what is the size of int type in C (it can be changed between 32 or 64 bits by compiler flag), while normally it is 32 bit for GCC/LLVM. So, compiling of Fortran-app+Tracy using this compiler will not produce a binary which will work correctly if int type is 64 bit long in Fortran side and Tracy was compiled with GCC/LLVM.

This seems to me like a defect in the compiler? I would expect the compiler to follow the 64 bit data model of the platform it targets, instead of randomly selecting a model that will produce binaries that do not match the ABI.

@foxtran
Copy link
Contributor Author

foxtran commented Jan 2, 2025

This seems to me like a defect in the compiler?

It is a feature. It switches between LP64/ILP64 models for both, Fortran and C, languages and it does not care about linking with binaries produced by other compilers.

@foxtran foxtran mentioned this pull request Jan 3, 2025
3 tasks
@wolfpld wolfpld merged commit 277555a into wolfpld:master Jan 9, 2025
6 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.

C API: avoid usage of int type
2 participants