Misc. changes to support CodeGenerator factory allocation #5566
Conversation
Clearing the way for a more generic, cross-CodeGenerator function. Signed-off-by: Daryl Maier <[email protected]>
0xdaryl
requested review from
aviansie-ben,
charliegracie,
fjeremic,
knn-k,
Leonardo2718,
mstoodle,
rwy7 and
youngar
as code owners
|
@Leonardo2718 : I'd appreciate a review from you please. |
|
Obviously a few wrinkles to work out on the OMR project side before this is un-WIPed... |
|
@genie-omr build all |
* Create a new CodeGenerator constructor that takes a TR::Compilation parameter. The constructor hierarchy only contains member initialization lists. A mirrored hierarchy was created to permit downstream projets to adapt to the hierarchy changes. The default constructors will eventually be deleted and replaced with private empty stubs. This constructor will eventually be invoked via a static factory function. * Create a CodeGenerator::initialize() method to be called after object construction to initialize the CodeGenerator object. The initialize() functions take the place of the original constructor bodies and must be invoked in the same order. Signed-off-by: Daryl Maier <[email protected]>
This is needed to prepare for the JitBuilder and TestCompiler CodeGenerators being removed. Signed-off-by: Daryl Maier <[email protected]>
The implementation was empty and provided no value over the default OMR CodeGenerator. Signed-off-by: Daryl Maier <[email protected]>
The implementation was empty and provided no value over the default OMR CodeGenerator. Signed-off-by: Daryl Maier <[email protected]>
|
@genie-omr build all |
|
Un-WIPing as tests have passed. |
0xdaryl
changed the title
| OMR::X86::CodeGenerator::initialize(TR::Compilation *comp) | ||
| OMR::X86::CodeGenerator::initializeX86(TR::Compilation *comp) |
There was a problem hiding this comment.
Why is this different than the rest?
There was a problem hiding this comment.
This API already existed in the X86 CodeGenerator constructor prior to this PR. I renamed it to initializeX86 to distinguish it from the name of the initialize function name I want to use for the purposes of the CodeGenerator constructor factory.
initializeX86 (as it is now called) was introduced at some point in the past to common up some initialization between the AMD64 and IA32 code generators. It is called from somewhere in the middle of the current constructors. I'm not sure why the constructors weren't simply modified to do this more naturally, but that's the state of the world at the moment.
I think a follow-on to this factory work is to change the x86 CodeGenerator hierarchy to initialize itself using the initialize() functions introduced by this PR. This might need some careful thinking and inspection as there are likely ordering requirements that would change given where initializeX86 is presently being called. I will open a follow-up issue for it.
There was a problem hiding this comment.
Ok, thanks for explaining. I was wondering why we couldn't just push this function into the common x86 CodeGenerator from which we derive the ADM64 and i386 versions. The ordering explains why that may not be possible or needs to be thought about. Thanks!
Issue: #3235