Initial Commit of the MicroJIT Project

[Scott-Young-6746]

MicroJIT is a work-in-progress templated JIT compiler that is a part of a research project between IBM and UNB. This PR is the initial contribution of MicroJIT.

Given an openj9-openjdk-jdk8 repository with support for the --enable-microjit flag, these commits will allow someone to test the current partial implementation of MicroJIT which can compile relatively few methods, however, it is asynchronous and does not prevent TR from compiling.

Compiling this project without the --enable-microjit flag in a supporting openj9-openjdk-jdk8, or in a repository without support for --enable-microjit, does not compile code associated with the MicroJIT project.

When compiling with the --enable-microjit flag in a supporting openj9-openjdk-jdk8 this code modifies the BytecodeInterpreter and the CompilationInfoPerThreadBase to allow methods to be compiled with the prototypical MicroJIT, and execute those compiled methods, without impeding the interpreter's ability to maintain an accurate count for TRs compilation threshold.

This PR also contains additions to several files in openj9/runtime/compiler to support MicroJIT compilation and MetaData structures to support both method compilation and execution of compiled methods.

See also:
ibmruntimes/openj9-openjdk-jdk8#413
Issue #9582

Signed-off-by: Scott Young [email protected]

[dsouzai]

This is a massive PR. Could this be broken up into smaller pieces? Also is there an issue outlining the general design?

[andrewcraik]

This definitely warrants and issue with a design. FYI @mstoodle since this likely is big enought to trigger a higher level eclipse process.

[0xdaryl]

Perhaps also introducing this work at one of the upcoming OpenJ9 community calls.

[Scott-Young-6746]

This definitely warrants and issue with a design. FYI @mstoodle since this likely is big enought to trigger a higher level eclipse process.

I had not thought of that, I'll begin creating the issue immediately and link it in the PR 👍

[mstoodle]

yes, the size may warrant a CQ be filled out, as we get closer to the final state of the PR.

[Scott-Young-6746]

This is a massive PR. Could this be broken up into smaller pieces? Also is there an issue outlining the general design?

An issue has been created and mentioned in the PR. Thank you for the feedback 😄

[andrewcraik]

I think the discussion should move over to the design issue (which needs a lot more details on the proposal) and probably discussion on a community call before a contribution of this size and complexity can be reviewed. @Scott-Young-6746 is the commit history as clean as it could be? It would be nice to have logical commits for the places where the code integrates into OpenJ9 as well as for the various parts of the microjit. At present it seems more like a stream of evoluation which is going to make the review harder since everyone has to look at everything...

@0xdaryl I am guesisng we are going to need some manual ECA verification on this because there are multiple commit authors on this code...

[Scott-Young-6746]

@Scott-Young-6746 is the commit history as clean as it could be? It would be nice to have logical commits for the places where the code integrates into OpenJ9 as well as for the various parts of the microjit. At present it seems more like a stream of evoluation which is going to make the review harder since everyone has to look at everything...

Thank you for the feedback Andrew. Are there any resources you can point me to that would illustrate how to modify the commit history?

I also received a suggestion late in the creation of the PR to wrap my code in #if defined(J9VM_OPT_MICROJIT) to make it easier to separate my changes from a VM that isn't using MicroJIT. Will that complicate the process of fixing the commit history?

[keithc-ca]

The line-end is missing here.

[Scott-Young-6746]

Thank you, I've updated all file-endings to have the required new-line at EOF.

[keithc-ca]

Why is this not conditional on J9VM_OPT_MICROJIT?

[Scott-Young-6746]

Thank you for catching this, it has been moved into a guarded area.

[keithc-ca]

Formatting: space after if (throughout).

[Scott-Young-6746]

I have updated the formatting of all if statements added by us in this file. Thank you for catching this!

[keithc-ca]

More parentheses required:

    value = ((trThreshold - mjitThreshold) << 1) | 1;

[Scott-Young-6746]

Updated now, thank you for catching this.

[keithc-ca]

What is 8 here? Does it assume a 64-bit target?

[Scott-Young-6746]

We are starting with x86-64, however this should not have been a magic value and we do want to make supporting other architectures easier. I added a method for getting the pointer size from the MicroJIT code generator.

[keithc-ca]

What's the value of this definition? Why not just use struct J9JITConfig?

[Scott-Young-6746]

I think this was a hold-over from the original attempt to port the old MicroJIT. I've removed it and replaced usage with the struct J9JITConfig as suggested.

[keithc-ca]

The only remaining change to this file is the copyright date: please revert that - no changes are required here.

[keithc-ca]

I think this change should be conditional on defined(J9VM_OPT_MICROJIT).

[Scott-Young-6746]

I've replaced it with an if defined() else endif section. This might need to be removed later though; as far as I know JBreturnToMicroJIT was for the old MicroJIT and nothing is using it anymore that I know of.

[0xdaryl]

I just took a quick glance over the code so I haven't digested this technically yet. However, one question I do have is other than the compilation control aspects, is there any sharing between the OpenJ9 JIT (i.e., what you find in the compiler directory) and your MicroJIT? Put another way, should microjit be a sibling of compiler rather than a child? My initial feel is that a sibling relationship is more appropriate as it is nearly a stand-alone component and perhaps in the future there could be some JVM environments that can only afford to include a microjit. Perhaps this is a variant of the question @andrewcraik has already raised around interaction with existing technologies in the JVM.

[andrewcraik]

Even if it is standalone in terms of operation of the template part could we share assets from the codegens (like the instruction encoding etc) to save duplication? Could we share some of the linkage implementation? GC mode support for things like read/write barriers?

It would also be good if we could standardize control options and logging formats/styles where reasonable. I would love an overview of where the current version is at. Again, my goal here is to lower the barrier to entry. We could almost even think of the microjit as another opt level below noOpt perhaps?

[Scott-Young-6746]

I just took a quick glance over the code so I haven't digested this technically yet. However, one question I do have is other than the compilation control aspects, is there any sharing between the OpenJ9 JIT (i.e., what you find in the compiler directory) and your MicroJIT? Put another way, should microjit be a sibling of compiler rather than a child? My initial feel is that a sibling relationship is more appropriate as it is nearly a stand-alone component and perhaps in the future there could be some JVM environments that can only afford to include a microjit. Perhaps this is a variant of the question @andrewcraik has already raised around interaction with existing technologies in the JVM.

Thanks for the feedback Daryl! We do currently use the J9JITConfig, and methods supporting MicroJIT have been added to MetaData creation. We do have our own space for building supporting calls to allow the GC to work, but this area has a lot of work left to do. We will likely be moving towards using the existing Linkage and Properties in TR for MicroJIT as it might help with porting later.

[Scott-Young-6746]

Even if it is standalone in terms of operation of the template part could we share assets from the codegens (like the instruction encoding etc) to save duplication? Could we share some of the linkage implementation? GC mode support for things like read/write barriers?

It would also be good if we could standardize control options and logging formats/styles where reasonable. I would love an overview of where the current version is at. Again, my goal here is to lower the barrier to entry. We could almost even think of the microjit as another opt level below noOpt perhaps?

Thanks for taking a look at this Andrew! I'll put a quick overview of the project's progress so far here:

Currently the project supports compiling a limited number of leaf methods.
Specifically, we do not yet support floating point operation, stack manipulation operations (e.g. the pop and dup bytecodes), or constant operations (e.g. iconst_0).

We have implemented and tested addition, subtraction, and multiplication of int and long variables and also support setting and loading static variables, though our getstatic and putstatic support is limited to fields that are resolvable at compile-time at this moment.

[Scott-Young-6746]

Even if it is standalone in terms of operation of the template part could we share assets from the codegens (like the instruction encoding etc) to save duplication? Could we share some of the linkage implementation? GC mode support for things like read/write barriers?

It would also be good if we could standardize control options and logging formats/styles where reasonable. I would love an overview of where the current version is at. Again, my goal here is to lower the barrier to entry. We could almost even think of the microjit as another opt level below noOpt perhaps?

A fairly large number of bytecodes not in the PR are being tested and will be PRed once we get everything here up to code. On our internal build we are able to compile methods with the following bytecodes:

• aload N
• aload_(0|1|2|3)
• astore N
• astore_(0|1|2|3)
• iload N
• iload_(0|1|2|3)
• istore N
• istore_(0|1|2|3)
• lload N
• lload_(0|1|2|3)
• lstore N
• lstore_(0|1|2|3)
• fload N
• fload_(0|1|2|3)
• fstore N
• fstore_(0|1|2|3)
• dload N
• dload_(0|1|2|3)
• dstore N
• dstore_(0|1|2|3)
• putfield[4]
• getstatic
• putstatic
• iadd
• isub
• imul
• iand
• ior
• ixor
• idiv[1]
• ladd
• lsub
• lmul
• land
• lor
• lxor
• return
• invokestatic
• iconst_(m1|0|1|2|3|4|5)
• lconst_(0|1)
• fconst_(0|1|2)
• bipush
  [1]: We are currently trying to fix a bug related to the hardware interupt handler for divide-by-zero.
  [4]: Further testing required, also trying to find a way to ensure that GC Heap isn't being corrupted.

We are also trying to clean up as much of the debugging infrastructure so that it is in one place instead of peppered throughout the code.

We currently use the Compilation object for acquiring some meta-data for compilation.

We use a copy&patch approach to templates, which are all written in NASM assembly at the moment.

MicroJIT relies on TR for some data structures and is called by TR in the CompilationThread.cpp, so I don't know how we could cleanly separate the two unless we took the common components used by both and lifted them to a separate folder. Something like:

openj9
+--runtime
|  +--compilers
|  |  +--common
|  |  +--testarossa
|  |  +--MicroJIT

[Scott-Young-6746]

Issue #9714 details a missing component of MicroJIT, namely support for OSR.

OSR support is not planned for this PR, however, MicroJIT should not cause unexpected behavior when OSR is enabled.

[keithc-ca]

This is likely to elicit a warning from the compiler: the cast simultaneously changes both the width and signed-ness.

[keithc-ca]

I'd like to see better formatting consistency; this method uses three styles:

  TR_ResolvedMethod * compilee
  TR_MethodMetaData* metaData
  J9Method *method

Semantically, the * binds more tightly to the parameter or local name, so the first and third styles are less mis-leading. Pick one (not the second style) and be consistent.

[keithc-ca]

typo: acces -> access

[keithc-ca]

What is the range of values for size? If it might be as large as 64, then useRAX must be wider than 2 bits.

[Scott-Young-6746]

Thank you for the review Keith. This function is called from a function which is checking the parameters that are passed via type signature. It's converting bytes to slots so the range should be {8,16}. Though I am interested in knowing if there are any special cases which might not conform to that on the JIT side.

[Scott-Young-6746]

Thank you for the review Keith. This function is called from a function which is checking the parameters that are passed via type signature. It's converting bytes to slots so the range should be {8,16}. Though I am interested in knowing if there are any special cases which might not conform to that on the JIT side.

[keithc-ca]

I'm not sure the space savings here is worth the added complexity: Why not just use uint8_t for fields like useRAX? The name could be improved the 'use' prefix implies 'boolean' to me.

[keithc-ca]

There's no need to use the comma operator here, and the statements should be on separate lines:

  *(uint32_t *)cursor = e->_instructionStartPC;
  cursor += 4;

Likewise, several places below.

[Scott-Young-6746]

Thanks again for the review Keith. This code was created because we needed a function to work on the MJIT_ExceptionTableEntryIterator. There is a nearly identical one used by TR above which was used as a starting point for this. The above lines of code were copied from that location.

Should I leave this code a is, modify both, or only make the change in the createMJITExceptionTable version?

[keithc-ca]

I'll let someone more familiar with the existing code comment on whether it should be updated, but my suggestion is to avoid the comma operator in new code.

[keithc-ca]

Every file should end with a final new-line.

[Scott-Young-6746]

This PR has been rebased and updated with changes to address the previous review by @keithc-ca

[andrewcraik]

First meeting to discuss microjit in OpenJ9 Friday 19 June 2020 1pm EST on https://ibm.webex.com/join/ajcraik all are welcome
Agenda hosted in https://ibm.box.com/s/gyb8ld476ja2542uyytjr5p4no7mamy4
Call will use my webex: https://ibm.webex.com/join/ajcraik
All are welcome and please feel free to edit the box document to add additional items to the agenda.

[andrewcraik]

The first meeting to discuss the contribution of MicroJIT to the OpenJ9 project has concluded. Overall it was a very positive and informative discussion - thank you everyone who participated and @Scott-Young-6746 for the overview of the current compiler design.

We have agreed to have another call on July 3 at 1pm EST continue discussions. In the meantime I will enumerate here some of the specific areas of integration where the MicroJIT developers are looking to get some specific technical help with integration:

• compilation control (@mpirvu @DanHeidinga @vijaysun-omr) - specifically avoiding the need for an extra2 field using some kind of GCR-like self counting mechanism in the microJIT code and also how the microjit fits with the OpenJ9 optimization levels etc
• linkage (@0xdaryl @andrewcraik @liqunl @fjeremic ) - looking to try and make the microJIT follow existing private linkage conventions to let existing tooling work with the compiled bodies more easily
• stackwalker (@gacholio @DanHeidinga) - looking to make the compiled bodies integrate with these existing pieces of infrastructure

We have not had a discussion around testing, but we do need to consider how the template JIT is tested both at a unit and integration level (@fjeremic @Leonardo2718 @smlambert @LinHu2016 )

There is probably also a need for some documentation of the design in a markdown document to summarize some of the overview provided, but that can wait until the integration discussion/design has progressed.

I will post a link to the follow-on agenda once I have found a home for it as well as a posting to the recording of this meeting on the openj9 youtube channel once it is ready. We will also look to create a microjit channel on the openj9 community slack for further specific technical discussions to avoid this issue getting too large since there are a number of things to be worked through in preparation for a merge.

[vijaysun-omr]

Regarding the compilation control question, I am wondering if there is some indication as to the relative compilation (CPU) time and scratch memory taken for micro JIT compiles in relation to cold and warm compilations in the full JIT. Perhaps @Scott-Young-6746 or @mpirvu have or can collect some data to give some insight that may help with the thought process on how to view these compiles in the overall picture.

[harpreetbamrah]

hey @dsouzai, the latest commits address the review points raised, please take a look.
@0xdaryl, it should be good now for a full review.
fyi @mpirvu,
Thanks!

[dsouzai]

I didn't go through the codegen/optimizer changes, but the infra and control updates look good.