Skip to content

Commit 9c99d01

Browse files
Copilotjakobbotsch
andauthored
Clarify JIT-EE API position/ordering in add-new-jit-ee-api skill; revert instructions change
Co-authored-by: jakobbotsch <7887810+jakobbotsch@users.noreply.github.com>
1 parent 780e9ef commit 9c99d01

2 files changed

Lines changed: 7 additions & 8 deletions

File tree

.github/instructions/jit.instructions.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,3 @@ applyTo: "src/coreclr/jit/**"
77
## Code Review Guidelines
88

99
- **Do not routinely request new targeted tests for pure refactors, mechanical cleanups, or other non-behavioral JIT codebase improvements.** JIT changes alter global codegen and are often validated by existing end-to-end suites and differential testing, so additional per-PR regression tests may not be needed for changes that do not affect observable behavior. However, targeted tests are appropriate and should be encouraged for bug fixes, changes with a clear repro, observable behavior changes, and previously untested edge cases that are not already covered by existing tests.
10-
- **Do not request that new JIT-EE (JIT-VM) interface methods be added at a specific position or in a particular ordering.** New JIT-EE methods can be added anywhere in the interface with any ordering without issues, so do not ask for them to be placed at the end, near similar signatures, or in any other particular location.

.github/skills/add-new-jit-ee-api/SKILL.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ CORINFO_METHOD_HANDLE getUnboxedEntry(CORINFO_METHOD_HANDLE ftn, bool* requiresI
2727
+CORINFO_METHOD_HANDLE getUnboxedEntry(CORINFO_METHOD_HANDLE ftn, bool* requiresInstMethodTableArg);
2828
```
2929

30-
Insert the new API definition without removing any existing entries, placing it near similar signatures.
30+
Insert the new API definition without removing any existing entries. The position and ordering of the new entry does not matter, so place it wherever is convenient (e.g. near similar signatures).
3131

3232
2. Invoke `<repo_root>/src/coreclr/tools/Common/JitInterface/ThunkGenerator/gen.sh` script
3333
(or `<repo_root>/src/coreclr/tools/Common/JitInterface/ThunkGenerator/gen.bat` on Windows) to update auto-generated files.
3434
Use the correct directory for the script to run.
3535

36-
3. Open `<repo_root>/src/coreclr/inc/corinfo.h` and add the new API inside `class ICorStaticInfo` class as the last member. Example:
36+
3. Open `<repo_root>/src/coreclr/inc/corinfo.h` and add the new API inside `class ICorStaticInfo` class. The position and ordering of the new method does not matter; adding it as the last member is fine. Example:
3737

3838
```diff
3939
+ virtual CORINFO_METHOD_HANDLE getUnboxedEntry(
@@ -42,7 +42,7 @@ Use the correct directory for the script to run.
4242
+ ) = 0;
4343
```
4444

45-
4. Open `<repo_root>/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs` and add the new API in the end of `class CorInfoImpl` class declaration. Use `<repo_root>/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs` to inspect how type parameters look like for C# for the newly added API since it is expected to be auto-generated there by the gen.sh(bat) script. Example:
45+
4. Open `<repo_root>/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs` and add the new API to `class CorInfoImpl` class declaration. The position and ordering of the new method does not matter; adding it at the end is fine. Use `<repo_root>/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs` to inspect how type parameters look like for C# for the newly added API since it is expected to be auto-generated there by the gen.sh(bat) script. Example:
4646

4747
```diff
4848
+ private CORINFO_METHOD_STRUCT_* getUnboxedEntry(CORINFO_METHOD_STRUCT_* ftn, ref bool requiresInstMethodTableArg)
@@ -55,7 +55,7 @@ Use the correct directory for the script to run.
5555

5656
Implement the API if asked, leave the NotImplementedException() otherwise.
5757

58-
5. Open `<repo_root>/src/coreclr/vm/jitinterface.cpp` and add a dummy implementation at the file's end. Example:
58+
5. Open `<repo_root>/src/coreclr/vm/jitinterface.cpp` and add a dummy implementation. The position and ordering of the new method does not matter; adding it at the file's end is fine. Example:
5959

6060
```diff
6161
+CORINFO_METHOD_HANDLE CEEInfo::getUnboxedEntry(
@@ -106,10 +106,10 @@ Add a new entry to the `LWM` list. Example:
106106
```
107107

108108
NOTE: Use upper-case for the first letter of the API name here.
109-
Add the new record after the very last LWM one.
109+
The position and ordering of the new record does not matter; adding it after the last LWM one is fine.
110110

111111
* `<repo_root>/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h`:
112-
Define 3 methods in this header file inside `class MethodContext` class (at the end of its definition).
112+
Define 3 methods in this header file inside `class MethodContext` class. The position and ordering of the new methods does not matter; adding them at the end of its definition is fine.
113113

114114
The methods are prefixed with `rec*` (record), `dmp*` (dump to console) and `rep*` (replay). Example
115115

@@ -125,7 +125,7 @@ Now add a new element to `enum mcPackets` enum in the same file. Example:
125125
```
126126

127127
* `<repo_root>/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp`:
128-
Add the implementation of the 3 methods to `methodcontext.cpp` at the end of it.
128+
Add the implementation of the 3 methods to `methodcontext.cpp`. The position and ordering of the new methods does not matter; adding them at the end of it is fine.
129129
Consider other similar methods in the file for reference. Do not change implementations of other methods in the file. Example:
130130

131131
```diff

0 commit comments

Comments
 (0)