[PATCH] D54143: [WIP, RISCV] Generate address sequences suitable for mcmodel=medium

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 added a comment.

> My only problem with that approach is that it seems wrong to expand PseudoLLA the same way I am expanding PseudoAddrPCRel, IE allowing the AUIPC operand to be decided by codegen.

I'm not sure to follow here.

I think @jrtc27 means that, instead of adding a new `PseudoAddrPCRel` and select it from a `WrapperPCRel`, we could select `PseudoLLA` and then expand it in `RISCVExpandPseudoInsts`.

That said, I presume at some point we will want to add codegen for GOT-addressing. We have a few options here but if we reuse the `WrapperPCRel` and we use a different target flag to record that this is a GOT-relocation, then the expansion of `PseudoLLA` in the codegen flow and the asm parser flow will be different (the latter always doing `%pcrel_hi` while the former might be able to do both `%pcrel_hi` / `%got_pcrel_hi`). This does not seem ideal to me. Adding another target-specific DAG (e.g. `WrapperGOTRel`) node is workable but feels unnecessary.

I'd lean towards having `PseudoAddrPCRel` because it avoids conflating the two entities due to having slightly different treatment (I'm aware in an earlier comment of mine I said using `PseudoLLA` made sense but now I'm less sure).

Maybe I'm misunderstanding the issue here.



================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:335
 
+SDValue getTargetNode(GlobalAddressSDNode *N, SDLoc DL, EVT Ty,
+                      SelectionDAG &DAG, unsigned Flags) {
----------------
Can these helper functions be made `static`?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54143/new/

https://reviews.llvm.org/D54143