[PATCH] D55303: [RISCV] Add lowering of addressing sequences for PIC

[Lewis Revill via Phabricator via llvm-commits]

lewis-revill marked an inline comment as done.
lewis-revill added inline comments.


================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:353
+template <class NodeTy>
+SDValue RISCVTargetLowering::getAddrPIC(NodeTy *N, SelectionDAG &DAG,
+                                        bool UseGOT, unsigned Flags) const {
----------------
jrtc27 wrote:
> lewis-revill wrote:
> > jrtc27 wrote:
> > > Can we combine this with `getAddr`, pushing the common `isPositionIndependent()` check inside `getAddr`? Then the `UseGOT` flag becomes `NonLocal` or similar (or alternatively invert it and make it an `IsLocal` flag), and happens to only influence code generation for PIC. This should simplify all its uses.
> > I'm not sure this is a good idea? We'd still have to check in `lowerGlobalAddress` for GOT, and I feel leaving `getAddr` to check `isPositionIndependent` seems misleading, and the other option is to end up checking `isPositionIndependent` twice for this path, and gaining nothing in the process. It's certainly doable though, if people think it is clearer.
> No you don’t, all you need to do is check `shouldAssumeDSOLocal` and pass that to `getAddr` as an `IsLocal` flag. The fact that the flag is only actually used on the PIC branch of `getAddr` is an implementation detail that `lowerGlobalAddress` doesn’t need to know about.
I tried this and it did look cleaner so I implemented it. Thanks for the suggestion.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55303