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

[Xiangling Liao via Phabricator via llvm-commits]

Xiangling_L added inline comments.
Herald added a subscriber: wuzish.


================
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));
----------------
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.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1767
+            TargetLoweringObjectFileXCOFF::getStorageClassForGlobal(GO);
+        GOSym->setStorageClass(SC);
+        MCSectionXCOFF *Csect = OutStreamer->getContext().getXCOFFSection(
----------------
Since this is the case where one MCSectionXCOFF has only one MCSymbolXCOFF, when we want to get `SC`, we can do `MCSymbolXCOFF->getContainingCsect()`? (Please correct me if I am wrong) If so, do we still want to bother to set `SC` for MCSymbolXCOFF in this case?


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-FuntionDS.ll:1
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+
----------------
To generate assembly from IR, please add `-verify-machineinstrs` and cpu level `-mcpu=pwr7`.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-FuntionDS.ll:3
+
+ at foo_ptr = global void (...)* @foo, align 4
+declare void @foo(...) #0
----------------
You can peel off extra useless info like `align 4`, `#0` etc. from testcase to make it cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71144