Use jit interface thunkgen tech for more files #45044
Conversation
Dotnet-GitSync-Bot
added
the
area-CodeGen-coreclr
|
@dotnet/jit-contrib |
There was a problem hiding this comment.
Can you leave breadcrumbs here and in the other newly auto-generated files to hiht at how they can be regenerated?
There was a problem hiding this comment.
Yes, please include pointers to the instructions for re-generation
There was a problem hiding this comment.
https://github.com/dotnet/runtime/blob/master/docs/project/updating-jitinterface.md is the current doc if changes are needed there.
|
@BruceForstall @sandreenko think you should look this over too. |
Why is that? That's very unfortunate. I think depending on the PAL headers would be ok, but it would be really nice to avoid linking with the PAL. I was hoping that the number of places we link the PAL will decrease over time... |
There was a problem hiding this comment.
nit? Looks like you could save lots of duplicated code by extracting a function to output function signatures, and function argument call lists
There was a problem hiding this comment.
Seems like you should have at least minimal argument validation here, like checking the number of arguments, and a "usage" message that would also serve as a short description of what this tool is doing.
There was a problem hiding this comment.
Nit: name these paths like ..\..\..\..\..\..\..\ etc?
There was a problem hiding this comment.
question: why was it renamed?
There was a problem hiding this comment.
CorInfoException is an enum type in corinfo.h
- Removes duplicate copy of jit ee api version guid - Sets up for making jit interface wrapper actually implement the jit interface Update generater to match previous manual modification Use ICorJitInfo interface for jit thunk directly instead of relying on parallel definition to match up - Found a bug in handling of TryResolveToken Generate api names Generate jit api wrapper from thunk generator Generate icorjitinfoimpl.h spmi counter shim icorjitinfo superpmi-shim-simple Update linux script Fix utf8 char out marshalling Builds on Linux but doesn't work Compile with PAL Respond to code review feedback
davidwrighton
force-pushed
the
better_thunk_gen
branch
from
c0819a1 to
6d957aa
Compare
|
Updated with code review feedback. I've stepped back from requiring the PAL, as that was causing odd problems. I may address that in a future change. |
| } | ||
| virtual unsigned int getMethodAttribs( | ||
| void* ftn) | ||
| { |
There was a problem hiding this comment.
The generator is ugly enough as it is. Unless there is pushback, I'd prefer to leave the indentation.
| @@ -0,0 +1,28 @@ | |||
|
|
|||
There was a problem hiding this comment.
Doesn't this file also need a license notice?
| 0x65a1, | ||
| 0x487a, | ||
| {0x85, 0x53, 0xc8, 0x45, 0x49, 0x6d, 0xa9, 0x01} | ||
| }; No newline at end of file |
There was a problem hiding this comment.
nit: missing trailing end-of-line character?
src/coreclr/src/inc/corinfo.h
Outdated
| }; | ||
|
|
||
| #include "jiteeversionguid.h" | ||
| ////////////////////////////////////////////////////////////////////////////////////////////////////////// |
There was a problem hiding this comment.
Seems like the "END JITEEVersionIdentifier" comment needs to be in the jiteeversionguid.h file.
We probably should have big, unmiss-able, comments at the top of all the JIT-EE interface files pointing to the JIT-EE version, but that's somewhat unrelated; this system only works when code reviewers know to ask for an interface GUID change.
Maybe the jiteeversionguid.h file should also point to the updating-jitinterface.md documentation.
With dotnet#45044 it moved from corinfo.h to jiteeversionguid.h
With dotnet#45044 it moved from corinfo.h to jiteeversionguid.h
With #45044 it moved from corinfo.h to jiteeversionguid.h
ghost
locked as resolved and limited conversation to collaborators
7 participants
Uh oh!
There was an error while loading. Please reload this page.