[PATCH] D13659: Implement .reloc (constant offset only) with support for R_MIPS_NONE and R_MIPS_32.

[David Majnemer via llvm-commits]

majnemer added a comment.

What happens when you do:

  .text
  .long .text+2
  .reloc 0, R_MIPS_32, .text+3


================
Comment at: lib/MC/MCParser/AsmParser.cpp:368
@@ -367,3 +367,3 @@
     DK_ERR, DK_ERROR, DK_WARNING,
-    DK_END
+    DK_END, DK_RELOC
   };
----------------
It'd be nicer if this could be closer to `DK_VALUE`.  I'd just leave it on it's own line.

================
Comment at: lib/MC/MCParser/AsmParser.cpp:488-489
@@ -487,1 +487,4 @@
 
+  // ".reloc"
+  bool parseDirectiveReloc(SMLoc DirectiveLoc);
+
----------------
Could this be moved near the handler for `.value` ?

================
Comment at: lib/MC/MCParser/AsmParser.cpp:4611-4614
@@ +4610,6 @@
+
+  // We can only deal with constant expressions at the moment.
+  int64_t OffsetValue;
+  if (!Offset->evaluateAsAbsolute(OffsetValue))
+    return Error(OffsetLoc, "expression is not a constant value");
+
----------------
dsanders wrote:
> Do you know any cases where the addend isn't supported? The GAS documentation says that ELF REL targets such as 386 don't support addends but I'm finding they work. Similarly, addends are also working for MIPS O32 which is also a REL target.
NT targets use REL relocations and they work with mingw assemblers.  Perhaps the comment is out of date.

================
Comment at: lib/Target/Mips/MCTargetDesc/MipsFixupKinds.h:27
@@ +26,3 @@
+    // Branch fixups resulting in R_MIPS_NONE.
+    fixup_Mips_None = FirstTargetFixupKind,
+
----------------
Would it be more consistent to name this `fixup_Mips_NONE` ?


http://reviews.llvm.org/D13659