[PATCH] D78045: [XCOFF][AIX] Fix getSymbol to return the correct qualname when necessary

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/include/llvm/Target/TargetLoweringObjectFile.h:245
 
+  /// Targets that have special convention for their symbols could use 
+  /// this hook to return a specialied symbol.
----------------
Add "a" before "special".


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1888
+
+  // We always use qualname symbol for the GV that represents
+  // a declaration, a function descriptor, or a common symbol.
----------------
Add "a" before "qualname". Replace "the" with "a" before "GV".


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1906
+  // For all other cases, return the unqualified name.
+  // This could change for GV that is a GlobalVariable when we decide to support
+  // -fdata-sections.
----------------
Add "a" before "GV".

Let's indicate the specific reason why we expect that this would change when `-fdata-sections` is specified (namely that we could avoid having label symbols if the linkage name is applied to the csect symbol).  User-specified section names can either cause such cases or get in the way of them.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-lower-comm.ll:5
+; RUN: llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc-ibm-aix-xcoff -filetype=obj -o %t.o < %s
+; RUN: llvm-readobj -t %t.o | FileCheck --check-prefix=SYM %s
+
----------------
The object-file testing is not enough to check against the existence of additional symbols whose unqualified name is `common`. It is probably sufficient if we just verify that the relocation names the defined `common[RW]`.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-lower-comm.ll:52
+; SYM-NEXT:       Index: [[#INDX+3]]
+; SYM-NEXT:       ContainingCsectSymbolIndex: 2
+; SYM-NEXT:       ParameterHashIndex: 0x0
----------------
This should use a FileCheck variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78045