[PATCH] D46989: [RISCV] Separate base from offset in lowerGlobalAddress (no peephole)

[Alex Bradbury via Phabricator via llvm-commits]

asb accepted this revision.
asb added a comment.
This revision is now accepted and ready to land.

Thanks Sameer, this looks good to me. It would be worth adding some sort of comment to lowerGlobalAddress about the decision to emit a separate ADD node rather than folding the offset into the global address.



================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:300
     report_fatal_error("Unable to lowerGlobalAddress");
 
+  SDValue GAHi = DAG.getTargetGlobalAddress(GV, DL, Ty, 0, RISCVII::MO_HI);
----------------
It would be good to add a comment here documenting the decision not to fold-the offset into the global address. e.g. "In order to maximise the opportunity for common subexpression elimination, emit a separate ADD node for the global address offset instead of folding it in the global address node. Later peephole optimisations may choose to fold it back in when profitable."


https://reviews.llvm.org/D46989