eclipse / omr

Eclipse OMR™ Cross platform components for building reliable, high performance language runtimes
http://www.eclipse.org/omr
Other
940 stars 394 forks source link

x86 codegen should try to use OMR::InstOpCode::Mnemonic #2126

Open fjeremic opened 6 years ago

fjeremic commented 6 years ago

As described in https://github.com/eclipse/omr/pull/2114#issuecomment-350306364 the x86 codegen currently uses its own enum to store the mnemonic values. It also overloads the setOpCodeValue function with this enum type. This means that the base OMR::Instruction cannot call self()->setOpCodeValue because the self() will delegate to the OMR::X86::Instruction class and it has an overloaded setOpCodeValue function.

This is an issue if other codegens wish to extend setOpCodeValue. Here is a travis build with the failure: https://travis-ci.org/eclipse/omr/jobs/313537837

/home/travis/build/eclipse/omr/compiler/codegen/OMRInstruction.cpp:120:29: note: candidate is:
In file included from /home/travis/build/eclipse/omr/compiler/x/codegen/Instruction.hpp:25:0,
                 from /home/travis/build/eclipse/omr/compiler/x/codegen/OMRCodeGenerator.hpp:56,
                 from /home/travis/build/eclipse/omr/compiler/x/amd64/codegen/OMRCodeGenerator.hpp:36,
                 from /home/travis/build/eclipse/omr/jitbuilder/codegen/JBCodeGenerator.hpp:36,
                 from /home/travis/build/eclipse/omr/jitbuilder/codegen/CodeGenerator.hpp:26,
                 from /home/travis/build/eclipse/omr/compiler/codegen/OMRInstruction.cpp:22:

/home/travis/build/eclipse/omr/compiler/x/codegen/OMRInstruction.hpp:98:18: note: TR_X86OpCodes OMR::X86::Instruction::setOpCodeValue(TR_X86OpCodes)
    TR_X86OpCodes setOpCodeValue(TR_X86OpCodes op) { return _opcode.setOpCodeValue(op); }
                  ^
/home/travis/build/eclipse/omr/compiler/x/codegen/OMRInstruction.hpp:98:18: note:   no known conversion for argument 1 from ‘TR::InstOpCode::Mnemonic’ to ‘TR_X86OpCodes’
fjeremic commented 6 years ago

@0xdaryl I'm not sure how OMR issues are tracked. Is ZenHub used at all? This will probably need proper tags added. @0dvictor FYI.