Modify __int64 definition in PAL to match the OS definition #77056
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change modifies the definition of __int64 and thus of many other types defined on the basis of it to match the OS definitions. This ensures that we can use these types in interfaces between code in coreclr and various PALs that are compiled against OS headers. The key issue was that we were defining __int64 for 64 bit OSes as long while Unix defines it as long long. The size of those types is the same on Unix, but they are different and result in different mangling of C++ names.
jkotas
reviewed
Oct 14, 2022
|
cc @AaronRobinsonMSFT This contributes to Win32 PAL elimination. |
shushanhf
reviewed
Oct 19, 2022
| @@ -195,12 +195,7 @@ extern "C" { | |||
| // they must be either signed or unsigned) and we want to be able to use | |||
| // __int64 as though it were intrinsic | |||
|
|
|||
| #ifdef HOST_64BIT | |||
There was a problem hiding this comment.
There is an error after deleting this on LoongArch64.
In file included from /home/qiao/work_qiao/runtime/src/coreclr/debug/di/platformspecific.cpp:37:
/home/qiao/work_qiao/runtime/src/coreclr/debug/di/loongarch64/cordbregisterset.cpp:272:18: error: cannot initialize a variable of type 'ULONG64 *'
(aka 'unsigned long long *') with an rvalue of type 'SIZE_T *' (aka 'unsigned long *')
ULONG64* pSrc = &m_rd->A0;
^ ~~~~~~~~~
1 error generated.
There was a problem hiding this comment.
@jkotas
Why define the ULONG64 as unsigned long long ?
While define SIZE_T as unsigned long ?
There was a problem hiding this comment.
this has been special cased to MacOS only in this PR: #77268, so guessing that should solve your issue?
There was a problem hiding this comment.
this has been special cased to MacOS only in this PR: #77268, so guessing that should solve your issue?
Yes, it's ok for LA64 now.
Thanks.
mangod9
pushed a commit
to mangod9/runtime
that referenced
this pull request
Nov 10, 2022
…7056) * Modify __int64 definition in PAL to match the OS definition This change modifies the definition of __int64 and thus of many other types defined on the basis of it to match the OS definitions. This ensures that we can use these types in interfaces between code in coreclr and various PALs that are compiled against OS headers. The key issue was that we were defining __int64 for 64 bit OSes as long while Unix defines it as long long. The size of those types is the same on Unix, but they are different and result in different mangling of C++ names. * Fix coreclr tests build * Fix comment on #endif in jit.h * Reflect PR feedback * Fix jit source formatting * Fix FreeBSD build
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change modifies the definition of
__int64and thus of many other types defined on the basis of it to match the OS definitions. This ensures that we can use these types in interfaces between code in coreclr and various PALs that are compiled against OS headers.The key issue was that we were defining
__int64for 64 bit OSes as long while Unix defines it aslong long. The size of those types is the same on Unix, but they are different and result in different mangling of C++ names.The changes I had to make in runtime were due to the fact that size_t and ssize_t are not defined as __int64 anymore due to the change and many places relied on the fact those matched.
The StressLog tools were defining couple of types including
size_ton its own, which collided with the new definitions. I've fixed it by including thestdint.hinstead of defining the types manually.