[PATCH] D154854: Use empty symbol name for XCOFF text csect

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-pgo-function-sections.ll:123
 ; OBJ64-NEXT: 0000000000000000         *UND*	0000000000000000 external_func
-; OBJ64-NEXT: 0000000000000000 l       .text	0000000000000000 .text
+; OBJ64-NEXT: 0000000000000000 l       .text	0000000000000000 
 ; OBJ64-NEXT: 0000000000000000 g       .text	0000000000000033 .func1
----------------
stephenpeckham wrote:
> hubert.reinterpretcast wrote:
> > It is somewhat unfortunate that the default tool outputs for empty names is especially bad, but I don't want to hold up this patch on changing that.
> A change to llvm-objdump that prints a visible string for empty names could be done in another patch.
Sounds good. Such a future patch should probably update all of the cases changed here too (although most of them will continue to pass even without such a future update).


================
Comment at: llvm/test/CodeGen/PowerPC/aix-text.ll:31-32
+; CHECKFS32: 00000000 l       .text  00000000 (idx: {{[[:digit:]]*}}) [PR]
+; CHECKFS32-NEXT: 00000000 g       .text  {{([[:xdigit:]]{8})}} (idx: {{[[:digit:]]*}}) .text
+; CHECKFS32-NEXT: {{([[:xdigit:]]{8})}} g       .text  {{([[:xdigit:]]{8})}} (idx: {{[[:digit:]]*}}) .text2
+
----------------
I expect we will see these named with the storage mapping class?


================
Comment at: llvm/test/CodeGen/PowerPC/aix-text.ll:35-36
+; CHECKFS64: 0000000000000000 l       .text  0000000000000000 
+; CHECKFS64-NEXT: 0000000000000000 g       .text  {{([[:xdigit:]]{16})}} (idx: {{[[:digit:]]*}}) .text
+; CHECKFS64-NEXT: {{([[:xdigit:]]{16})}} g       .text  {{([[:xdigit:]]{16})}} (idx: {{[[:digit:]]*}}) .text2
+
----------------
Same comment.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-text.ll:2
+; RUN: llc -filetype=obj --function-sections -mtriple powerpc-ibm-aix-xcoff -o %t.o < %s
+; RUN: llvm-objdump --syms %t.o | FileCheck %s
+; RUN: llc -filetype=obj --function-sections -mtriple powerpc64-ibm-aix-xcoff -o %t64.o < %s
----------------
hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > Since this is a new test, I think we should use `--symbol-description` (same for other case below).
> @stephenpeckham, while the case where `text()` is defined needs `function-sections` to exhibit the failure, there is another case where `text()` is an external reference that does not need `function-sections`.
> 
> I suggest adding that additional test as well.
> 
This test (where `text` is defined as a function) needs `function-sections` to exhibit the problem.
A separate test (different source), where `text` is an undefined function being referenced, exhibits the problem with or without `function-sections`. In other words, a different test file is needed to capture the additional case.


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

https://reviews.llvm.org/D154854