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

Daniel Sanders daniel.sanders at imgtec.com
Tue May 19 08:22:02 PDT 2015


> Comments addressed. Added llvm-rtdyld checker test, marked MCJIT and OrcMCJIT PIC tests as XFAIL only for mips32 architecture, and added an error report when using N32/O32 ABI's with Mips64 triple.


There's still several triple checks that should be ABI checks. Using the triple for more than distinguishing the Mips target from the others is incorrect.

Also, the llvm-rtdyld test doesn't seem to cover many relocations. Could you expand on it?


REPOSITORY
  rL LLVM

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp:777
@@ -776,2 +776,3 @@
       continue;
-    resolveRelocation(RE, Value);
+    if (Arch == Triple::mips64el || Arch == Triple::mips64)
+      // MIPS64 N64 ABI is specific in resolving relocations since
----------------
Target triple -> ABI checks.

Only N64 should take this path.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:526-529
@@ +525,6 @@
+  // next type of the same relocation entry.
+  useCalculatedValue = !applyRelocation;
+  applyRelocation = r_type2 == ELF::R_MIPS_NONE;
+  resolveMIPS64Relocation(Section, RE.Offset, Value, r_type, RE.Addend,
+                          RE.SectionID, RE.SymOffset);
+
----------------
This particular overload of resolveMIPS64Relocation is only called from this function. So if we rename it to evaluateMIPS64Relocation(), stop it applying the reloc, and have it take/return the current calculated value we can use it as per this pseudo-code:
  int64_t calculatedValue = evaluateMIPS64Relocation(Section, RE.Offset, Value, r_type, RE.Addend, RE.SectionID, RE.SymOffset, calculatedValue);
  if (r_type2 == ELF::R_MIPS_NONE)
    applyMIPS64Relocation(Section.Address + RE.Offset, calculatedValue);
This is neater than passing values in side-channels such as member variables.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:578-579
@@ +577,4 @@
+  case ELF::R_MIPS_32: {
+    uint32_t *TargetPtr = reinterpret_cast<uint32_t *>(Section.Address
+                                                       + Offset);
+    calculatedValue = Value + Addend;
----------------
Endian bug. You can't use reinterpret_cast since the host and target may have different endians (particularly for test-cases). Use the endian-aware writeBytesUnaligned() or similar instead. Likewise for the other instances below.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:1289
@@ -1058,1 +1288,3 @@
     }
+  } else if (Arch == Triple::mips64el || Arch == Triple::mips64) {
+    uint32_t r_type = RelType & 0xff;
----------------
Triple checks -> ABI checks.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:1603-1606
@@ -1355,4 +1602,6 @@
   case Triple::systemz:
+  case Triple::mips64el:
+  case Triple::mips64:
     Result = sizeof(uint64_t);
     break;
   case Triple::x86:
----------------
Not 100% sure about this one, but I think it's only correct for N64 due to pointer size differences. If this is the case, then we need an ABI check

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:1667
@@ -1417,1 +1666,3 @@
     memset(Addr, 0, TotalSize);
+    if (Arch == Triple::mips64el || Arch == Triple::mips64) {
+      unsigned AbiVariant;
----------------
Triple check -> ABI check.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:1670-1671
@@ +1669,4 @@
+      Obj.getPlatformFlags(AbiVariant);
+      if ((AbiVariant & ELF::EF_MIPS_ABI2) ||
+          (AbiVariant & ELF::EF_MIPS_ABI_O32))
+        llvm_unreachable("N32 ABI and O32 ABI are not supported on Mips64");
----------------
mips64/mips64el and O32 is not an error case, it's valid. Admittedly it doesn't work in LLVM at this point (this is a bug) so maybe including O32 in this error is ok for now. Could you add a comment explaining that.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h:124-132
@@ -116,1 +123,11 @@
 
+  // Apply relocation in instruction (Mips64 specific)
+  bool applyRelocation;
+
+  // True if using the result of the previous relocation type (Mips64 specific)
+  bool useCalculatedValue;
+
+  // The result of the previous relocation type (Mips64 specific)
+  int64_t calculatedValue;
+
+  // A map from section to a GOT section that has entries for section's GOT
----------------
These three shouldn't be members variables. See below.

http://reviews.llvm.org/D9667

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






More information about the llvm-commits mailing list