[PATCH] D23568: [RISCV 10/10] Add common fixups and relocations

[David Majnemer via Phabricator via llvm-commits]

majnemer added inline comments.


================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:557-561
+  const MCSymbolRefExpr *SE = dyn_cast<MCSymbolRefExpr>(Expr);
+  if (SE) {
+    // It's a simple symbol reference with no addend.
+    return true;
+  }
----------------
This would be more clear as:
  // It's a simple symbol reference with no addend.
  if (isa<MCSymbolRefExpr>(Expr))
    return true;


================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:560
+    // It's a simple symbol reference with no addend.
+    return true;
+  }
----------------
Is it your intention to return `true` with `Kind` equal to `RISCVMCExpr::VK_RISCV_Invalid`?


================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:582
+  // on here than we can deal with.
+  auto AddendExpr = dyn_cast<MCConstantExpr>(BE->getRHS());
+  if (!AddendExpr)
----------------
Didn't your just `isa<>`? I think this can be `cast<>`.


https://reviews.llvm.org/D23568