[PATCH] D71144: Not emit correct Assembly for Global Function pointer initiated with function.

[David Tenty via Phabricator via llvm-commits]

daltenty added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1762
+  if (const GlobalObject *GO = dyn_cast<GlobalObject>(CV)) {
+    if (isa<Function>(GO)) {
+      MCSymbolXCOFF *GOSym = cast<MCSymbolXCOFF>(getSymbol(GO));
----------------
Xiangling_L wrote:
> For the testcase you listed in the summary, for the second part: 
> 
> ```
> void bar() {
> return ;
> }
> void (*bar_ptr1)() = &bar;
> ```
> 
> I think:
> 
> ```
> bar_ptr1:
> .long bar
> ```
> 
> is also a correct output, since function `bar` has definition in current TU, so `bar` here refers to the label within `.csect bar{DS}`. I am okay with either `.long bar` or `.long bar{DS}`. But either way, I think you probably should update the summary to avoid confusion.
I'd like to continue to use `.long bar{DS}`  to keep consistent with what's in getMCSymbolForTOCPseudoMO, as you noted in the summary.

But I agree you should probably update the summary to clarify it, starting with a description of the problem we trying to solve rather than just the expected output, which can be confusing if you don't already know why we expect what we do.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1772
+            SectionKind::getMetadata());
+        GOSym->setContainingCsect(Csect);
+      }
----------------
Aside: This would probably also benefit from the refactor suggested in D71125


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-FuntionDS.ll:2
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+
+ at foo_ptr = global void (...)* @foo, align 4
----------------
Is there a reason not to check the 64-bit case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71144