[PATCH] D72056: [RISCV] Generate PIC address sequence for medany -fno-pic

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 2 inline comments as done.
MaskRay added a subscriber: bsdjhb.
MaskRay added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp:680
+  if (MF->getTarget().isPositionIndependent() ||
+      MF->getTarget().getCodeModel() != CodeModel::Small) {
     const auto &STI = MF->getSubtarget<RISCVSubtarget>();
----------------
jrtc27 wrote:
> This is confusing and differs from when you invoke the assembler manually, even via the Clang driver with the right code model specified. Any `la`s there will be assembled as `AUIPC`/`ADDI`. I am of the view it was a mistake to make `la`'s behaviour conditional on PICness and it should have always used the GOT, but this is what we have.
> I am of the view it was a mistake to make la's behaviour conditional on PICness and it should have always used the GOT, but this is what we have.

I agree with you. `la`'s behavior should not be conditional on PIC. RISC-V probably should have a GOT relaxation mechanism. See my comment at https://github.com/riscv/riscv-elf-psabi-doc/issues/126


================
Comment at: llvm/lib/Target/TargetMachine.cpp:192
       return false;
+    // RISC-V non-small code models prefer avoiding copy relocations.
+    if (TT.isRISCV() && getCodeModel() != CodeModel::Small)
----------------
jrtc27 wrote:
> Are we sure we want to do this and take the performance hit over GCC due to the extra level of indirection on every single extern global access? If this is the solution to take for extern weak, I think we should limit it to just that and have a separate discussion about non-extern-weak.
I'd like to know more about `-mcmodel=medany -fno-pic`'s use cases. I guess @bsdjhb's FreeBSD kernel use case can probably be met by `-mcmodel=medany -fpie`.

I think there is no precedent which differs extern-strong and extern-weak. If GOT relaxation works, we can make this unconditional.

  // RISC-V prefer avoiding copy relocations.
  if (TT.isRISCV()
    return false;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72056