[PATCH] D143708: [RISCV] Support emulated TLS

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4655
+
   assert(N->getOffset() == 0 && "unexpected offset in global node");
 
----------------
Should this assert apply? Given it's true of ELF TLS I'd expect it of emulated TLS too?


================
Comment at: llvm/test/CodeGen/RISCV/emutls.ll:2-5
+; RUN: llc < %s -emulated-tls -mtriple=riscv32 -relocation-model=pic \
+; RUN:     | FileCheck -check-prefix=RV32 %s
+; RUN: llc < %s -emulated-tls -mtriple=riscv64 -relocation-model=pic \
+; RUN:     | FileCheck -check-prefix=RV64 %s
----------------
Triple should come first and `< %s` should come last


================
Comment at: llvm/test/CodeGen/RISCV/emutls.ll:109
+
+; RV32:        .data
+; RV32:        .globl __emutls_v.external_y
----------------
Please indent all these to look like how the assembly is formatted, same as is done for matching code


================
Comment at: llvm/test/CodeGen/RISCV/emutls.ll:117
+; RV32-NEXT:   .word __emutls_t.external_y
+; RV32-NOT:    __emutls_v.external_x:
+; RV32:        .section .rodata,
----------------
Why all these NOTs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143708