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

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 17 05:10:16 PDT 2017


asb added inline comments.


================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:560
+    // It's a simple symbol reference with no addend.
+    return true;
+  }
----------------
majnemer wrote:
> Is it your intention to return `true` with `Kind` equal to `RISCVMCExpr::VK_RISCV_Invalid`?
It's not, good catch. Initialising Kind to RISCVMCExpr::VK_RISCV_None seems to be the correct thing to do.


================
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)
----------------
majnemer wrote:
> Didn't your just `isa<>`? I think this can be `cast<>`.
I think this is the first time we check for MCConstantExpr. Unless I'm misunderstanding you, I don't think this has to be a dyn_cast


https://reviews.llvm.org/D23568





More information about the llvm-commits mailing list