[PATCH] D101875: [RISCV] Prefer to lower MC_GlobalAddress operands to .Lfoo$local

Mon May 10 14:12:17 PDT 2021

jrtc27 added inline comments.

================
Comment at: llvm/test/CodeGen/RISCV/elf-preemption.ll:1-3
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -relocation-model=static < %s | FileCheck %s --check-prefix=STATIC
+; RUN: llc -mtriple=riscv64 -relocation-model=pic < %s | FileCheck %s --check-prefix=PIC
----------------
luismarques wrote:
> Nit: we generally add both riscv32 and riscv64 (if applicable, generally naming the prefix appropriately). I'm not quite sure where to draw the line to ensure that doesn't become silly (@jrtc27 opinions?), but in this case it looks like the codegen might be the same for both subarchs, so it might not even require any additional clutter.
> Also, split the long RUN lines, like in other tests.
GOT loads will differ in width, and stack spilling will differ in the few tests that end up doing that. RV32 tests should be redundant, but they should be so quick it does no harm to add them.

================
Comment at: llvm/test/CodeGen/RISCV/elf-preemption.ll:138-139
+
+;; bl .Ldsolocal_func$local either resolves to a constant at assembly time
+;; or produces a relocation which can potentially cause a veneer.
+define dso_local void @call_dsolocal_func() nounwind {
----------------
luismarques wrote:
> Is a veneer the same thing as a thunk?
Veneer is the NIH term Arm use for range-extension thunks. This comment is meaningless for RISC-V; the bl is AArch64 and RISC-V has no veneers.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101875