Omr: Centralize the definition of Op Code lists in OMR

Created on 28 Oct 2019  路  7Comments  路  Source: eclipse/omr

Hi,

It appears at the moment that the Opcodes appear in multiple places in OMR - as listed in https://github.com/eclipse/omr/blob/master/compiler/il/OMRILOpCodesEnum.hpp. I had a quick look at the way this is done, and it seemed to me (at first glance) that the multiple definitions could be controlled in one place by adopting a simple strategy as follows:

  1. Create or amend a header file so that the entries in that file are macros of the form:

OPCODE(....)

Each line would represent an opcode and the parameters would be the required values at different places.

  1. Then at each location where the Opcodes are currently listed, the code would include the header file and define an appropriate macro expansion.

I am not sure of the impact on OpenJ9 but I would imagine that OpenJ9 can be left as is, as it would see the same definitions as before (after expansion). Separately changes could be done to OpenJ9.

architecture review pending compiler enhancement

Most helpful comment

For a concrete proposal for how this could look, I'd suggest taking a peek at the SpiderMonkey Opcodes.h, the usage guideline and the various uses in the codebase.

This pattern has been replicated throughout the SpiderMonkey codebase and is in my opinion a really effective strategy for managing tabular data that I'd love to see used in OMR.

I also think it should be possible to build extensible enums atop this with minimal effort

All 7 comments

FYI @mstoodle @0xdaryl @vijaysun-omr @Leonardo2718 @fjeremic

For a concrete proposal for how this could look, I'd suggest taking a peek at the SpiderMonkey Opcodes.h, the usage guideline and the various uses in the codebase.

This pattern has been replicated throughout the SpiderMonkey codebase and is in my opinion a really effective strategy for managing tabular data that I'd love to see used in OMR.

I also think it should be possible to build extensible enums atop this with minimal effort

For a concrete proposal for how this could look, I'd suggest taking a peek at the SpiderMonkey Opcodes.h, the usage guideline and the various uses in the codebase.

Looks like a very clever solution to the problem.

That's a neat solution to encapsulate all opcodes in a single file! :)

I am not still not sure, though, whether it solves the extensibility problem. For example, imagine this centralized solution is in place. When OpenJ9 (or any other client) add their own opcode, they should still be careful with the order, right? Because if OMR adds an opcode, then OpenJ9 does that as well, the problem is that OpenJ9 now has to handle a merge conflict in this single file.

TL;DR: the order of opcodes is still enforced, isn't it?

@oneturkmen

That's a neat solution to encapsulate all opcodes in a single file! :)

I am not still not sure, though, whether it solves the extensibility problem. For example, imagine this centralized solution is in place. When OpenJ9 (or any other client) add their own opcode, they should still be careful with the order, right? Because if OMR adds an opcode, then OpenJ9 does that as well, the problem is that OpenJ9 now has to handle a merge conflict in this single file.

TL;DR: the order of opcodes is still enforced, isn't it?

So, the point of this design is to consolidate the opcode list in one place. With one source of truth, we can guarantee that any tables that get generated from the "master list" will all be in the same order. The exact order of the opcodes themselves doesn't (shouldn't?) matter. The only thing that matters is that the order is consistent across all tables.

By itself, this design does not solve the extensibility problem. However, that can probably be solved using an approach similar to our extensible enums. In that design, downstream projects would just "append" their own opcode master list to the one that's in OMR. Each project would then essentially get a different opcode table. So, the order of opcodes across projects doesn't matter. OMR can add new opcodes and downstream projects will just pick them up.

Does that answer your question?

@Leonardo2718 yup, thanks!

@dibyendumajumdar has not started yet (based on our discussion), so I would like to take a look into this issue and make an attempt to provide an implementation.

Was this page helpful?
0 / 5 - 0 ratings