[PATCH] D143708: [RISCV] Support emulated TLS

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

Didn't notice before, but why emutls_generic.ll rather than emutls.ll? The suffix doesn't add any value as far as I can see. It's as generic as any other RISC-V CodeGen test.



================
Comment at: llvm/test/CodeGen/RISCV/emutls_generic.ll:1
+; RUN: llc < %s -emulated-tls -mtriple=riscv64-gnu-linux -relocation-model=pic \
+; RUN:     | FileCheck -check-prefix=RISCV_64 %s
----------------
These should be bare-metal triples.


================
Comment at: llvm/test/CodeGen/RISCV/emutls_generic.ll:3
+; RUN:     | FileCheck -check-prefix=RISCV_64 %s
+; RUN: llc < %s -emulated-tls -mtriple=riscv64-gnu-linux -relocation-model=pic -O3 \
+; RUN:     | FileCheck -check-prefix=RISCV_64 %s
----------------
vit9696 wrote:
> jrtc27 wrote:
> > Why do we need optimisation tests?
> To make sure there are no oddities with the optimisation passes against emutls.
Well:
1. llc defaults to -O2
2. you could say that about anything, emutls isn't special
3. if it works with optimisations it'll almost certainly work without

Axe this.


================
Comment at: llvm/test/CodeGen/RISCV/emutls_generic.ll:20
+
+; NoEMU-NOT: __emutls
+
----------------
vit9696 wrote:
> jrtc27 wrote:
> > Why's that in an emutls test? And isn't that implied by our TLS tests not having anything to do with emutls in the generated code?
> It is not implied. Negative checking is mandatory for any good test, in my opinion.
It *is* implied. The non-emutls tests do not contain calls to anything emutls. Negative checking is *not* mandatory, in fact it's *bad* for a CodeGen test because it's a hand-written CHECK line that can't be automated. Plus CHECK-NOT can be quite hard to use correctly.

Axe this.


================
Comment at: llvm/test/CodeGen/RISCV/emutls_generic.ll:63
+; RISCV_64-NOT:    __emutls_v.external_x:
+; RISCV_64:        .section .r{{o?}}data,
+; RISCV_64-LABEL:  __emutls_t.external_y:
----------------
vit9696 wrote:
> jrtc27 wrote:
> > What on earth is .rdata?
> .rdata is used in PE/COFF instead of .rodata. As long as Microsoft adds RISC-V support this is how this code may be generated.
But you're generating code for ELF. ELF does not have .rdata.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143708