[PATCH] D98946: [AIX][TLS] Add support for TLS variables to XCOFF object writer

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-tls-xcoff-variables.ll:4
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff -filetype=obj -o %t.o < %s
+; RUN: llvm-readobj --section-headers %t.o | FileCheck --check-prefix=OBJ %s
+; RUN: llvm-readobj --syms %t.o | FileCheck --check-prefixes=SYMS,SYMS-DATASECT %s
----------------
change --check-prefix=OBJ to --check-prefix=SECTION ?


================
Comment at: llvm/test/CodeGen/PowerPC/aix-tls-xcoff-variables.ll:9
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff -data-sections=false -filetype=obj -o %t.o < %s
+; RUN: llvm-readobj --section-headers %t.o | FileCheck --check-prefix=OBJ %s
+; RUN: llvm-readobj --syms %t.o | FileCheck --check-prefixes=SYMS,SYMS-NODATASECT %s
----------------
ditto


================
Comment at: llvm/test/CodeGen/PowerPC/aix-tls-xcoff-variables.ll:11
+; RUN: llvm-readobj --syms %t.o | FileCheck --check-prefixes=SYMS,SYMS-NODATASECT %s
+; RUN: llvm-objdump -D --symbol-description %t.o | FileCheck --check-prefixes=OBJDUMP,OBJDUMP-NODATASECT %s
+
----------------
if do not use --symbol-description , I think llvm-objdump -D for -data-sections=on / off should be same.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-tls-xcoff-variables.ll:14
+; RUN: not --crash llc -mtriple powerpc64-ibm-aix-xcoff -filetype=obj < %s 2>&1 | FileCheck --check-prefix=XCOFF64 %s
+; RUN: not --crash llc -mtriple powerpc64-ibm-aix-xcoff -data-sections=false -filetype=obj < %s 2>&1 | FileCheck --check-prefix=XCOFF64 %s
+; XCOFF64: LLVM ERROR: 64-bit XCOFF object files are not supported yet.
----------------
since we do not support generating  64bits object file. I do not think we need two different not --crash llv for -data-sections on and off.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-tls-xcoff-variables.ll:517
+
+ at tls_global_int_external_val_initialized = thread_local global i32 1, align 4
+ at tls_global_int_external_zero_initialized = thread_local global i32 0, align 4
----------------
suggest to remove the external from @tls_global_int_external_val_initialized , it is not a external variable. 


================
Comment at: llvm/test/CodeGen/PowerPC/aix-tls-xcoff-variables.ll:518
+ at tls_global_int_external_val_initialized = thread_local global i32 1, align 4
+ at tls_global_int_external_zero_initialized = thread_local global i32 0, align 4
+ at tls_global_int_local_val_initialized = internal thread_local global i32 2, align 4
----------------
ditto


================
Comment at: llvm/test/CodeGen/PowerPC/aix-tls-xcoff-variables.ll:524
+ at tls_global_int_weak_val_initialized = weak thread_local global i32 1, align 4
+ at tls_global_int_external_uninitialized = external thread_local global i32, align 4
+ at tls_global_double_common_zero_initialized = common thread_local global double 0.000000e+00, align 8
----------------
change @tls_global_int_external_uninitialized to @tls_global_int_external ? 


================
Comment at: llvm/test/CodeGen/PowerPC/aix-tls-xcoff-variables.ll:526
+ at tls_global_double_common_zero_initialized = common thread_local global double 0.000000e+00, align 8
+ at tls_global_double_external_uninitialized = external thread_local global i64, align 8
+ at tls_global_long_long_internal_val_initialized = internal thread_local global i64 1, align 8
----------------
change @tls_global_double_external_uninitialized to  @tls_global_double_external


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98946