-
Notifications
You must be signed in to change notification settings - Fork 398
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
Implement z/OS XPLINK system linkage #3779
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI note to reviewers:
This is not the final linkage change I will be delivering. It is merely a good synchronization point for review and initial delivery. I have left many TODOs for myself which I will continue to work on in the background. The system linkage rabbit hole is very deep, and to avoid a super-bloated PR I'm trying to get this portion delivered first as I continue to work on it.
I'll continue adding self reviews to further explain some of the decisions made.
TR::Instruction* cursor = data.cursorInstruction; | ||
|
||
// TODO: We should be caching the PROC instruction as it's used in several places and is pretty important | ||
while (cursor && cursor->getOpCodeValue() != TR::InstOpCode::PROC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also want to consolidate this so it's the same on all platforms. I'm working on making such a change at the common level.
#define AggregatesPassedOnParmStack 0x400 | ||
#define AggregatesPassedInParmRegs 0x800 | ||
#define AggregatesReturnedInRegs 0x1000 | ||
// Available 0x2000 | ||
#define SmallIntParmsAlignedRight 0x4000 ///< < gprSize int parms aligned into the parmword | ||
#define ParmBlockRegister 0x8000 ///< Has a parameter block register: OS Linkage (non-Java) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only for z/OS OS linkage which doesn't exist in OMR, so deprecating it. We only support XPLINK, and currently have no plans to support other system linkages.
@@ -60,7 +60,6 @@ | |||
XHHR, // Exclusive OR High (High <- High) | |||
XHLR, // Exclusive OR High (High <- Low) | |||
XLHR, // Exclusive OR High (Low <- High) | |||
XPCALLDESC, // zOS-31 LE Call Descriptor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This used to be used for an "inline" XPLINK call descriptor. It is now replaced but a much more robust and documented XPLINKCallDescriptorSnippet
setIntegerArgumentRegister(0, TR::RealRegister::GPR2); | ||
setIntegerArgumentRegister(1, TR::RealRegister::GPR3); | ||
|
||
if (!comp()->getCurrentMethod()->isJ9()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@0xdaryl I didn't get to this here but I will be moving this out in my subsequent PR.
uint8_t* cursor = cg()->getBinaryBufferCursor(); | ||
|
||
// TODO: We should not have to do this here. This should be done by the caller. | ||
getSnippetLabel()->setCodeLocation(cursor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be fixing this as well at the common level.
FYI... Please see the Travis CI build failure. |
Ah, yes. I had to resolve a conflict with one of your recent commits in that file. I must have merged it incorrectly. I'll fix this first thing in the morning. |
d5ce5c9
to
0ddae4d
Compare
@genie-omr build all |
z/OS build failed due to @genie-omr build zos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the one tiny little bit I have knowledge about, I approve :) .
Maybe @joransiu could provide a review on the XPLINK aspects, please?
I'll approve once rebased. |
Deprecate `TRzOSSystemLinkageBase` and push it's APIs into `S390zOSSystemLinkage` since we only support a single z/OS system linkage. Signed-off-by: Filip Jeremic <[email protected]>
Signed-off-by: Filip Jeremic <[email protected]>
Signed-off-by: Filip Jeremic <[email protected]>
The `generateBinaryEncodingPrologue` has generic code for the following: 1. Creating the recompilation preprologue 2. Creating the method preprologue 3. Creating the method prologue On Z there is nothing specific to OpenJ9 about the above steps. In fact the steps are missing from OMR and hence no prologue/preprologue is generated of system linkages at the OMR level during binary encoding. This is a problem and the fix is to simply migrate this code over. Signed-off-by: Filip Jeremic <[email protected]>
The entry point marker is a 4-word struct right before the entry point of the method body which contains method metadata. [1] https://www.ibm.com/support/knowledgecenter/en/SSLTBW_2.1.0/com.ibm.zos.v2r1.ceev100/rtnly6.htm Signed-off-by: Filip Jeremic <[email protected]>
These labels mark the start and end of the entire method. They are convenience labels such that we always have an instruction at which to insert other instructions, thus effectively prepending instructions to the start of the instruction stream. Signed-off-by: Filip Jeremic <[email protected]>
This hash table is never referenced yet compilation time is wasted adding branches into the table. Remove it to improve compilation time and footprint. Signed-off-by: Filip Jeremic <[email protected]>
We have redundant loops which estimate the binary encoding length of an instruction. Because of the way we generate prologues and recompilation pre-prologues we may miss to estimate some instructions. Avoid this by only having one estimation loop which traverses all instructions and estimates them. Signed-off-by: Filip Jeremic <[email protected]>
Signed-off-by: Filip Jeremic <[email protected]>
Signed-off-by: Filip Jeremic <[email protected]>
Now that we have proper implementations of PPA1 and PPA2 we can generate both snippets as part of prologue creation and add the appropriate relocations in the Entry Point Marker in the function preprologue to correctly update the offset to PPA1. With this commit we have the bare minimum we need for this JIT generated method body to be XPLINK conformant. Signed-off-by: Filip Jeremic <[email protected]>
There is no reason to assert because of an unknown snippet type when tracing. Instead if we end up calling the base class virtual implementation we will print a sane default value indicating to the user that there is indeed a snippet there, however no custom printing logic has been implemented yet. Signed-off-by: Filip Jeremic <[email protected]>
Signed-off-by: Filip Jeremic <[email protected]>
The Linux and z/OS system linkage classes are large and complicated enough that they deserve to live within their own compilation units. This commit partitions up the two classes into: - SystemLinkageLinux.cpp - SystemLinkagezOS.cpp and updates the corresponding include files in the rest of the codebase. We take this opportunity to completely fold `S390SystemLinkage` as it really serves no purpose since the `SystemLinkage` class already exists in the `TR` namespace. Signed-off-by: Filip Jeremic <[email protected]>
The frame types always default to `standardFrame` and we currently do not handle other frame types, such as leaf frames for simplicity. This commit cleans up all related APIs which serve no purpose at the moment. Signed-off-by: Filip Jeremic <[email protected]>
This linkage strategy is no better than what we do by default. In fact it looks almost identical. We deprecate it here to simplify the implementation of the system linkage as this option provides us no benefit. Signed-off-by: Filip Jeremic <[email protected]>
Some APIs such as the creation of a prologue and epilogue are system linkage specific, depending on the calling convention of the system. Such APIs belong in the system linkage classes. Signed-off-by: Filip Jeremic <[email protected]>
With the introduction of four new APIs: - `fillGPRsInEpilogue` - `fillFPRsInEpilogue` - `spillGPRsInPrologue` - `spillFPRsInPrologue` We can migrate code from the the prologue and epilogue creation into the aforementioned APIs to simplify the implementation. There can be more work done within these functions, for example taking advantage of the GPR/FPR masks which will be handled in subsequent commits. Signed-off-by: Filip Jeremic <[email protected]>
Signed-off-by: Filip Jeremic <[email protected]>
The `preProcInstruction` is the instruction that was before the JIT to JIT entry point before the arguments are loaded from the stack. This is the instruction we want to use as the entry point into the method. Signed-off-by: Filip Jeremic <[email protected]>
Currently the z/OS XPLINK only supports 64-bit targets. We currently do not intend to support 31-bit XPLINK targets at the OMR level. There was some pre-existing support for floating point parameter descriptors which are required only for 31-bit to describe floating point arguments. We not longer need this support as the PPA1 structure does not support generating float parameter descriptors. Signed-off-by: Filip Jeremic <[email protected]>
We prepare the code to the be partitioned into the following functions: - `fillGPRsInEpilogue` - `fillFPRsInEpilogue` - `spillGPRsInPrologue` - `spillFPRsInPrologue` similarly to what we did on Linux. This will make the code much easier to understand across the two system linkages. Signed-off-by: Filip Jeremic <[email protected]>
- Create the new snippet - Migrate Call Descriptor generation function to the snippet - Document all the new APIs - Make use of the new snippet when generating the call outs - Clean up generation of Call Descriptor relocation - Remove XPLINKDESC, now unused pseudo-instruction - Enhance internal relocation class to be more generic Signed-off-by: Filip Jeremic <[email protected]>
These relocations can be used by other codegens as well. Document and expose them at the common level. Signed-off-by: Filip Jeremic <[email protected]>
Both linkages should look as identical as possible in terms of API definitions so as to make it easier for a reader to understand. We partition mainly the z/OS system linkage here so it looks more or less like the Linux system linkage. During this exercise we identify that quite some code related to prologue and epilogue creation is actually common, as is the filling and spilling of the registers. This opens up the doors for more commoning to happen at the common system linkage level, a change which will follow shortly. Signed-off-by: Filip Jeremic <[email protected]>
We may be dispatching a call to an XPLINK function from a non-XPLINK function, in which case we may not have an Entry Point Marker for the non-XPLINK body. We still however need the call descriptor to be present as the XPLINK body we are calling will return past the NOP descriptor. Signed-off-by: Filip Jeremic <[email protected]>
Signed-off-by: Filip Jeremic <[email protected]>
@genie-omr build all |
To get JitBuilder working on z/OS we need a proper z/OS system linkage in place, that means both frame shape and callout support. At the moment we have some half-baked solution in OMR which does not cut it, since we cannot even run the simplest JitBuilder test cases (see #3693).
This PR introduces minimal, but proper XPLINK support to OMR and lays out the groundwork for pushing Java-isms out of OMR and consolidating much of the system linkage support across Linux and z/OS. There are many changes in the PR, each of which deserves it's own commit. Each commit comment outlines the specific change made. The work is not done yet, but to avoid feature creep as this is a deep rabbit hole I've chosen this as the point where the code is good enough to be reviewed and contributed to the project while I continue to work on it in the background.
The following is a summary of some of the goals of this PR: