[PATCH] D144941: [m68k] Add TLS Support

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

Some minor comments



================
Comment at: llvm/lib/Target/M68k/M68kISelLowering.cpp:1509
+
+  llvm_unreachable("should not be here");
+}
----------------
Not a useful error message


================
Comment at: llvm/test/CodeGen/M68k/TLS/tlsgd.ll:11
+; CHECK-NEXT:    suba.l #4, %sp
+; CHECK-NEXT:    .cfi_def_cfa_offset -8
+; CHECK-NEXT:    lea (_GLOBAL_OFFSET_TABLE_ at GOTPCREL,%pc), %a0
----------------
Best to use nounwind in tests where you don't care about CFI but get them due to function calls


================
Comment at: llvm/test/CodeGen/M68k/TLS/tlsie.ll:10
+; CHECK-NEXT:  ; %bb.0: ; %entry
+; CHECK-NEXT:    suba.l #4, %sp
+; CHECK-NEXT:    .cfi_def_cfa_offset -8
----------------
Why do we need the stack adjustment? We're not passing any arguments to __m68k_read_tp. Or is this some stack alignment issue to compensate for the implicit push of the return address?


================
Comment at: llvm/test/CodeGen/M68k/TLS/tlsld.ll:8
+; CHECK-LABEL: get_addr:
+; CHECK:       .Lget_addr$local:
+; CHECK-NEXT:    .type .Lget_addr$local, at function
----------------
Teach ASM_FUNCTION_M68K_RE in UpdateTestChecks to filter these


================
Comment at: llvm/test/CodeGen/M68k/TLS/tlsle.ll:3-4
+; RUN: llc -mtriple=m68k -o - %s | FileCheck %s
+
+
+ at myvar = internal thread_local global i32 2, align 4
----------------



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

https://reviews.llvm.org/D144941