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

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/MC/MCObjectStreamer.cpp:447-448
@@ +446,4 @@
+  if (Assembler->getBackend().getFixupKind(Name, Kind)) {
+    if (Expr == nullptr)
+      Expr = MCSymbolRefExpr::create(getContext().createTempSymbol(), getContext());
+    DF->getFixups().push_back(MCFixup::create(OffsetValue, Expr, Kind, Loc));
----------------
I don't believe you have a test case which exercises this case.  I'm a little confused as to why we need to create a symbol here. GNU as doesn't seem to do this:
  $ cat t.s
  .text
  .long 0
  .reloc .text, R_X86_64_32
  $ as t.s -o t.o
  $ objdump -r t.o
  t.o:     file format elf64-x86-64

  RELOCATION RECORDS FOR [.text]:
  OFFSET           TYPE              VALUE                                                                                                                                                   
  0000000000000000 R_X86_64_32       *ABS*

================
Comment at: lib/MC/MCParser/AsmParser.cpp:4603
@@ -4596,1 +4602,3 @@
 
+bool AsmParser::parseDirectiveReloc(SMLoc DirectiveLoc) {
+  const MCExpr *Offset;
----------------
It might be nice to have a comment here describing the syntax using pseudo-EBNF.  We do this for some directives like `parseDirectiveRept` and it makes it a little easier to see the "big idea".

================
Comment at: lib/MC/MCParser/AsmParser.cpp:4628-4629
@@ +4627,4 @@
+    Lexer.Lex();
+    if (parseExpression(Expr))
+      return true;
+  }
----------------
I think we should `evaluateAsRelocatable` the `Expr` to sanity check the expression. Stuff like `.text + .text` should fail with a nice message.


http://reviews.llvm.org/D13659