[llvm-commits] [llvm] [PATCH] Mips JIT implementation

[Bruno Cardoso Lopes]

Hi Vladimir,

Thanks for working on this.

Sorry for the delay. When I apply the patch locally, the MIPS backend
compilation fails. You probably forgot to define stuff like
X86_64_JIT, as you can see in the X86 backend. Please, make sure this
patch will enable the compilation to work on a non-mips machine. Also,
some coding standarts are not being followed.

1) Indentation problems:

+  // FIXME: Move this to TargetJITInfo!
+    setRelocationModel(Reloc::Static); //force Static relocation

Align function call with the comments.

+class MipsCodeEmitter : public MachineFunctionPass {
+    MipsJITInfo *JTI;
+    const MipsInstrInfo *II;
+    const TargetData *TD;

Use only 2 spaces here.

Please read http://llvm.org/docs/CodingStandards.html for more details.

2) Regarding the way you are implementing the emitInstruction(...) method:

+enum InstrType {
+  arithm_6_5_5_5_6,
+  arithm_6_5_5_16, // addiu
+  load_store, // lw, sw
+  j,
+  jal,
+  jalr,
+  jr,
+  nop,
+  lui,
+  ret, // fake jr $ra
+  mul_div, // madd, maddu, msub, msubu, div, divu, mult, multu
+  shift, // sll, sra, srl
+  count_leading, // clo, clz
+  branch, // beq, bgez, bgezal, bgtz, blez, bltzal, bltz, bne
+  lea_addiu,
+  mf,
+  nothing,
+  mov_fp,
+  shift_6_5_5_5_6,
+  fp_6_5_5_5_6,
+  fp_6_5_5_5_3_2_2_4,
+  branch_cp
+};

Instead of using the enums above, it would be better if you do
something like we have today in the X86 backend. Take a look at the
namespace X86II in lib/Target/X86/X86InstrInfo.h. MIPS also has a few
instruction formats, you can emit instructions by matching their
format, using annotations in similar ways that X86 does for its
prefixes (see again the X86II), and the emitInstruction(...) would be
a lot more clean and smaller. For instructions that aren't easy to
directly emit only by knowing its format, you can add more enums as
X86 does. Besides that, the instructions in MipsInstrInfo.td should
contain as much information about the encoding as possible, so before
fixing a instruction encoding directly in emitInstruction(...), make
sure you can't handle that first in the instruction definition.

You probably want to set MIPS as having JIT in the makefile system,
look up for x86 and JIT there to find an example.

This is a great step. Please make the requested changes and send the
patches again, and if there are no objections from other developers, I
would be happy to commit when it gets in shape.

Thank you!

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc