[PATCH] D29938: [RISCV 16/n] Support and tests for a variety of additional LLVM IR constructs

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:152
+  if (!isPositionIndependent() && !Subtarget->is64Bit()) {
+    SDValue GAHi = DAG.getTargetExternalSymbol(Sym, Ty, RISCVII::MO_HI);
+    SDValue GALo = DAG.getTargetExternalSymbol(Sym, Ty, RISCVII::MO_LO);
----------------
reames wrote:
> I think this needs a comment.  Why is emitting the symbol twice the right thing to do?
As always, thanks for the feedback Philip. What comment would you like to see? Emitting two symbols (e.g. the "Hi" and "Lo" part) is common for fixed length ISAs. I want to make the code as accessible as possible, but explaining hi/lo relocations at each instance might be repetitive?


https://reviews.llvm.org/D29938