[PATCH] [Mips64] Add support for MCJIT for MIPS64r2 and MIPS64r6

[Vladimir Radosavljevic]

REPOSITORY
  rL LLVM

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:191
@@ -190,3 +190,3 @@
     : RuntimeDyldImpl(MemMgr, Resolver), GOTSectionID(0), CurrentGOTIndex(0) {}
-RuntimeDyldELF::~RuntimeDyldELF() {}
+RuntimeDyldELF::~RuntimeDyldELF() { SectionToGOTMap.clear(); }
 
----------------
dsanders wrote:
> Nit: Seems unnecessary. It's about to be destroyed anyway and we don't seem to be managing memory.
Done.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:536
@@ +535,3 @@
+  uint32_t RelType = r_type;
+  int64_t calculatedValue = evaluateMIPS64Relocation(Section, Offset, Value,
+                                                     RelType, Addend,
----------------
dsanders wrote:
> Nit: Variables should begin with a capital.
> 
> r_type/r_type2/r_type3 are arguably ok since they are trying to match member names of an ELF64 structure so there's no need to change those.
Done.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:561
@@ +560,3 @@
+
+  DEBUG(dbgs() << "resolveMips64Relocation, LocalAddress: 0x"
+               << format("%llx", Section.Address + Offset)
----------------
dsanders wrote:
> Nit: Doesn't match the function name (the 'resolve' and the 'Mips')
Done.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:579-581
@@ +578,5 @@
+  case ELF::R_MIPS_32:
+  case ELF::R_MIPS_64:
+    calculatedValue = Value + Addend;
+    break;
+  case ELF::R_MIPS_26:
----------------
dsanders wrote:
> Optional nit: Move the return into the cases. E.g:
>   case ELF::R_MIPS_64:
>     return Value + Addend;
Done.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:1035
@@ -817,1 +1034,3 @@
+      resolveMIPS64Relocation(Section, Offset, Value, Type, Addend, SymOffset,
+                              SectionID);
     break;
----------------
dsanders wrote:
> Nit:
>   else
>     llvm_unreachable("Mips ABI not handled");
Done.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:1277-1280
@@ +1276,6 @@
+  } else if (IsMipsN64ABI) {
+    uint32_t r_type = RelType & 0xff;
+    RelocationEntry RE(SectionID, Offset, RelType, Value.Addend);
+    if (r_type == ELF::R_MIPS_CALL16 || r_type == ELF::R_MIPS_GOT_PAGE
+        || r_type == ELF::R_MIPS_GOT_DISP) {
+      StringMap<uint64_t>::iterator i = GOTSymbolOffsets.find(TargetName);
----------------
dsanders wrote:
> Is it definitely only r_type that matters? Is it possible to have these relocs in r_type2 or r_type3?
Yes only r_type matters, and in these cases r_type2 and r_type3 are R_MIPS_NONE.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:1605
@@ -1362,2 +1604,3 @@
+      Result = sizeof(uint64_t);
     break;
   default:
----------------
dsanders wrote:
> Nit:
>   else
>     llvm_unreachable(...);
Done.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h:61
@@ +60,3 @@
+
+  void applyMIPS64Relocation(uint8_t *TargetPtr, int64_t calculatedValue,
+                             uint32_t Type);
----------------
dsanders wrote:
> Nit: calculatedValue should begin with a capital
Done.

http://reviews.llvm.org/D9667

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/