Adjust logic in cor.h, corhdr.h, corinfo.h, corjit.h and such so that they may be included without the PAL #46055
Conversation
|
@jkotas, @AndyAyersMS I put this together early in December, and I was curious if you like the ideas here. Personally, I found the volume of changes to be very large, and the final benefit, which was to make the jit interface dll a little more reliable to be not sufficiently motivating. Also, I see that it is a pathway to making the JIT completely divorced from the PAL (and as such able to tech such as the STL, etc) which might be beneficial, but I'm not sure how important that might be. Thoughts? |
|
I do like the idea of reducing reliance on Windows PAL and Windows specific constructs on Unix. I know that the changes are not always pretty, but I believe they are good for the long run. I am supportive of this change. |
src/coreclr/inc/corjit.h
Outdated
| @@ -144,6 +144,14 @@ enum CheckedWriteBarrierKinds { | |||
|
|
|||
| #include "corjithost.h" | |||
|
|
|||
| #if !defined(_MSC_VER) && !defined(__stdcall) | |||
There was a problem hiding this comment.
Can we delete stdcall and just depend on the build defaults? This is not a public header, so we do not need to make it work in the presence of arbitrary build defaults.
There was a problem hiding this comment.
Is it safe to assume that all users of the interface are compiled the same way (i.e., all assume stdcall by default, or all assume cdecl)? On Windows it was always best practice to be explicit about the calling convention of cross-module APIs.
There was a problem hiding this comment.
JIT/EE interface is not a public interface. All users of this interface live in this repo and are under our control. We compile everything with same compiler flags.
|
I like the change spirit too. The long term goal is to get rid of runtime dependency on Windows APIs emulation on Unix and my even longer term dream is to get rid of Windows types from the runtime completely. |
|
Agree, this seems worth doing. |
Convert most implementation to the standard types Rerun generation tool It actually builds! Fix ifdef Fix CorSigUncompressData duplication on Unix Remove pointless use of specstring.h header Packing doesn't need to be specified: specstrings tweaks stdcall tweak Use standard sized int types in more places Fix non-x64 arch issues Apply formatting patch
davidwrighton
force-pushed
the
usecorjit_h
branch
from
0846c50
to
b62d58d
Compare
…quire special definition in native code
davidwrighton
changed the title
|
@jkotas @AndyAyersMS I believe this is now ready for review |
|
@BruceForstall FYI |
There was a problem hiding this comment.
I like the change. It would be useful to have some guidance somewhere about what types to use where. E.g., when to use the stdint size-specified types and when to use the C++ defined types (char, int, long). It seems like an impossible (and undesirable?) goal to replace all types in the CLR with stdint types, and therefore we can always assume "int" is signed 32-bits and "long" is signed 64-bits, for example. If we have a goal of building where that (or some similar concept) is not true, then we need to know what the end goal is, and how to get there.
ghost
locked as resolved and limited conversation to collaborators
This was done via simple substitution of the standard sized integer types, and dealing with the fallout. It is tested by using the ICorJitInfo interface directly within the crossgen2 jitinterface thunk library.
Effort was made to use a minimum number of casts, as casts in such a large change are likely to be wrong somewhere.
In addition, a small amount of logic from the PAL headers were pulled into the proper header to allow compilation in the presence of some SAL annotation, and other small details.
Overall, I'm not entirely pleased with the changes, but I'm curious to see if they actually work everywhere.