[AIX] Avoid unset csect assert for functions defined after their use in TOC

[daltenty]

Summary

If a function is defined after it appears in a TOC expression, we may
try to access an unset containing csect when returning a symbol for the
expression.

[DiggerLin]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1740

for global ,I checked the gcc generated Functions Description as HIDDEN.
for static function, I checked both gcc and xlc generated Functions Description as HIDDEN.

[Xiangling_L]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1740

For static function, Digger is right, function descriptor symbol should have internal storage type.

But for external linkage function, I think what Digger mentioned I checked the gcc generated Functions Description as HIDDEN should be interpreted as GCC generates two symbols for function descriptor, one is .csect foo[DS], the SD symbol has C_HIDEXT storage type, the other one is foo as a label within the csect which has C_EXT SC? And the label one plays the important role in GCC case to expose foo's function descriptor when the address of function is taken.

So in LLVM, since we use the same symbol to represent the foo in .csect foo[DS] and the foo as label, I think the storage class here should follow the linkage type of function symbol, like:

const XCOFF::StorageClass SC =
            TargetLoweringObjectFileXCOFF::getStorageClassForGlobal(GO);

 MCSectionXCOFF *FnDescSec = OutStreamer->getContext().getXCOFFSection(
      CurrentFnDescSym->getName(), XCOFF::XMC_DS, XCOFF::XTY_SD,
      SC, SectionKind::getData());

[DiggerLin]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1905

can we implement as
if (GOKind.isText()) {

if (!XSym->hasContainingCsect()) {
   .......
   XSym->setContainingCsect(Csect);
 }

return XSym->getContainingCsect()->getQualNameSymbol();
}

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1740

I agree with @Xiangling_L. The storage class should match that of the function symbol itself.

[hubert.reinterpretcast]

This patch might need splitting. At least the commit subject should encompass the scope more. Once the tests are added for weak external or internal linkage, the "drive-by" change will be sizable.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1902–1912

The binding of "initialized" is unclear here. Do you mean "defined" functions?

1910

Note regarding the linkage here as well.

[daltenty]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1740

I agree. I originally intended to use getStorageClassForGlobal, but for some reason the XCOFF object file format document declares that CSects of mapping class have XMC_DS have storage class value of C_EXT.

As noted this doesn't make much sense, so I will update this as described.

1905

I'm thinking we may need to refactor how we setup containing csects for GlobalObjects in general. We end up duplicating the logic for what type of csect to create for a particular type of global in multiple places, and its not clear who owns setting up the containing csect (i.e. is it the first place we need the csect, or emitGlobalVariable and friends?).

Perhaps getSymbol() could setup the containing CSect if it doesn't exist. Or maybe some type of getOrCreateContainingCsect(GlobalObject) function is more appropriate.

Anyway, I'd like to address this in a follow on refactor patch, since this one is already getting large in scope.

[hubert.reinterpretcast]

The scope of the "drive-by" change is heading into redesign territory.

llvm/test/CodeGen/PowerPC/aix-func-dsc-gen.ll

59 ↗

(On Diff #232583)

This is not okay. There is already an external label definition with the same unqualified name. If we want to consider making the csect external, then we should stop creating the label. We will have to use a qualified name for the .globl in the assembly.

[daltenty]

In D71125#1784954, @hubert.reinterpretcast wrote:

The scope of the "drive-by" change is heading into redesign territory.

I agree, I'm going to split the function descriptor change out and adopt the current mapping class of C_HIDEXT for this change.

[daltenty]

• Remove function descriptor storage changes. They can best be reviewed in their own patch.

[daltenty]

• Update comment

[hubert.reinterpretcast]

@daltenty, please update the commit message to indicate the corrected scope. This LGTM insofar as it fits the current handling of function descriptors for which we have a definition in the current object.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

 }
 
 void PPCAIXAsmPrinter::SetupMachineFunction(MachineFunction &MF) {
   // Get the function descriptor symbol.
   CurrentFnDescSym = getSymbol(&MF.getFunction());
   // Set the containing csect.
   MCSectionXCOFF *FnDescSec = OutStreamer->getContext().getXCOFFSection(
       CurrentFnDescSym->getName(), XCOFF::XMC_DS, XCOFF::XTY_SD,
       XCOFF::C_HIDEXT, SectionKind::getData());
   cast<MCSymbolXCOFF>(CurrentFnDescSym)->setContainingCsect(FnDescSec);
 
   return AsmPrinter::SetupMachineFunction(MF);
 }
 
 void PPCAIXAsmPrinter::ValidateGV(const GlobalVariable *GV) {
   // Early error checking limiting what is supported.
   if (GV->isThreadLocal())
if (!XSym->hasContainingCsect()) {
           XSym->getName(), isa<Function>(GO) ? XCOFF::XMC_DS : XCOFF::XMC_UA,
           XCOFF::XTY_ER, SC, SectionKind::getMetadata());
       XSym->setContainingCsect(Csect);
     }
 
     return XSym->getContainingCsect()->getQualNameSymbol();
   }
 
-  // Handle initialized global variables.
-  if (GV) {
-    SectionKind GVKind = getObjFileLowering().getKindForGlobal(GV, TM);
+  // Handle initialized global variables and defined functions.
+  SectionKind GOKind = getObjFileLowering().getKindForGlobal(GO, TM);
 
+  if (GOKind.isText()) {
+    // If the MO is a function, we want to make sure to refer to the function
+    // descriptor csect.
+    return OutStreamer->getContext()
+        .getXCOFFSection(XSym->getName(), XCOFF::XMC_DS, XCOFF::XTY_SD,
+                         XCOFF::C_HIDEXT, SectionKind::getData())
+        ->getQualNameSymbol();
+  } else if (GOKind.isCommon() || GOKind.isBSSLocal()) {
     // If the operand is a common then we should refer to the csect symbol.
-    if (GVKind.isCommon() || GVKind.isBSSLocal()) {
-      MCSectionXCOFF *Csect = cast<MCSectionXCOFF>(
-          getObjFileLowering().SectionForGlobal(GV, GVKind, TM));
-      return Csect->getQualNameSymbol();
-    }
+    return cast<MCSectionXCOFF>(
+               getObjFileLowering().SectionForGlobal(GO, GOKind, TM))
+        ->getQualNameSymbol();
 
-    // Other global variables are refered to by labels inside of a single csect,
-    // so refer to the label directly.
-    return getSymbol(GV);
-  }
 
-  // If the MO is a function, we want to make sure to refer to the function
-  // descriptor csect.
-  return XSym->getContainingCsect()->getQualNameSymbol();
-}
-
 /// createPPCAsmPrinterPass - Returns a pass that prints the PPC assembly code
 /// for a MachineFunction to the given output stream, in a format that the
 /// Darwin assembler can deal with.
 ///
 static AsmPrinter *
 createPPCAsmPrinterPass(TargetMachine &tm,
                         std::unique_ptr<MCStreamer> &&Streamer) {
   if (tm.getTargetTriple().isMacOSX())

llvm/test/CodeGen/PowerPC/aix-xcoff-toc.ll

 @globa = common global i32 0, align 4
 
 @ptr = internal global void (...)* null, align 4
 
 declare void @foo()
 
 define void @bar() {
   %1 = alloca i8*, align 8
+  %2 = alloca i8*, align 8
   store i32 0, i32* @a, align 4
   store i64 0, i64* @b, align 8
   store i16 0, i16* @c, align 2
   store i32 0, i32* @globa, align 4
   store void (...)* bitcast (void ()* @bar to void (...)*), void (...)** @ptr, align 4
   store i8* bitcast (void ()* @foo to i8*), i8** %1, align 8
+  store i8* bitcast (void ()* @foobar to i8*), i8** %2, align 8
   ret void
 }
 
 ; We initialize a csect when we first reference an external global, so make sure we don't run into problems when we see it again.
 define void @bar2() {
   store i32 0, i32* @a, align 4
   store i64 0, i64* @b, align 8
   store i16 0, i16* @c, align 2
   ret void
 }
 
+define void @foobar() {
+  ret void
+}
+
 ; CHECK-NOT: .comm a
 ; CHECK-NOT: .lcomm a
 ; CHECK-NOT: .comm b
 ; CHECK-NOT: .lcomm b
 ; CHECK-NOT: .comm c
 ; CHECK-NOT: .lcomm c
 ; CHECK: .comm globa[RW],4,2
 ; CHECK32: .lcomm ptr,4,ptr[BS],2
 ; CHECK64: .lcomm ptr,8,ptr[BS],2
 ; CHECK:      .toc
 ; CHECK-NEXT: LC0:
 ; CHECK-NEXT: .tc   a[TC],a[UA]
 ; CHECK-NEXT: LC1:
 ; CHECK-NEXT: .tc   b[TC],b[UA]
 ; CHECK-NEXT: LC2:
 ; CHECK-NEXT: .tc   c[TC],c[UA]
 ; CHECK-NEXT: LC3:
 ; CHECK-NEXT: .tc   globa[TC],globa[RW]
 ; CHECK-NEXT: LC4:
 ; CHECK-NEXT: .tc   ptr[TC],ptr[BS]
 ; CHECK-NEXT: LC5:
 ; CHECK-NEXT: .tc   bar[TC],bar[DS]
 ; CHECK-NEXT: LC6:
 ; CHECK-NEXT: .tc   foo[TC],foo[DS]
+; CHECK-NEXT: LC7:
+; CHECK-NEXT: .tc   foobar[TC],foobar[DS]
 
 ; SYM:       File: {{.*}}aix-xcoff-toc.ll.tmp.o
 ; SYM:       Symbol {{[{][[:space:]] *}}Index: [[#INDX:]]{{[[:space:]] *}}Name: TOC
-; SYM-NEXT:    Value (RelocatableAddress): 0x8C
+; SYM-NEXT:    Value (RelocatableAddress): 0xA8
 ; SYM-NEXT:    Section: .data
 ; SYM-NEXT:    Type: 0x0
 ; SYM-NEXT:    StorageClass: C_HIDEXT (0x6B)
 ; SYM-NEXT:    NumberOfAuxEntries: 1
 ; SYM-NEXT:    CSECT Auxiliary Entry {
 ; SYM-NEXT:      Index: [[#INDX+1]]
 ; SYM-NEXT:      SectionLen: 0
 ; SYM-NEXT:      ParameterHashIndex: 0x0
 ; SYM-NEXT:      TypeChkSectNum: 0x0
 ; SYM-NEXT:      SymbolAlignmentLog2: 2
 ; SYM-NEXT:      SymbolType: XTY_SD (0x1)
 ; SYM-NEXT:      StorageMappingClass: XMC_TC0 (0xF)
 ; SYM-NEXT:      StabInfoIndex: 0x0
 ; SYM-NEXT:      StabSectNum: 0x0
 ; SYM-NEXT:    }
 ; SYM-NEXT:  }
 ; SYM-NEXT:  Symbol {
 ; SYM-NEXT:    Index: [[#INDX+2]]
 ; SYM-NEXT:    Name: a
-; SYM-NEXT:    Value (RelocatableAddress): 0x8C
+; SYM-NEXT:    Value (RelocatableAddress): 0xA8
 ; SYM-NEXT:    Section: .data
 ; SYM-NEXT:    Type: 0x0
 ; SYM-NEXT:    StorageClass: C_HIDEXT (0x6B)
 ; SYM-NEXT:    NumberOfAuxEntries: 1
 ; SYM-NEXT:    CSECT Auxiliary Entry {
 ; SYM-NEXT:      Index: [[#INDX+3]]
 ; SYM-NEXT:      SectionLen: 4
 ; SYM-NEXT:      ParameterHashIndex: 0x0
 ; SYM-NEXT:      TypeChkSectNum: 0x0
 ; SYM-NEXT:      SymbolAlignmentLog2: 2
 ; SYM-NEXT:      SymbolType: XTY_SD (0x1)
 ; SYM-NEXT:      StorageMappingClass: XMC_TC (0x3)
 ; SYM-NEXT:      StabInfoIndex: 0x0
 ; SYM-NEXT:      StabSectNum: 0x0
 ; SYM-NEXT:    }
 ; SYM-NEXT:  }
 ; SYM-NEXT:  Symbol {
 ; SYM-NEXT:    Index: [[#INDX+4]]
 ; SYM-NEXT:    Name: b
-; SYM-NEXT:    Value (RelocatableAddress): 0x90
+; SYM-NEXT:    Value (RelocatableAddress): 0xAC
 ; SYM-NEXT:    Section: .data
 ; SYM-NEXT:    Type: 0x0
 ; SYM-NEXT:    StorageClass: C_HIDEXT (0x6B)
 ; SYM-NEXT:    NumberOfAuxEntries: 1
 ; SYM-NEXT:    CSECT Auxiliary Entry {
 ; SYM-NEXT:      Index: [[#INDX+5]]
 ; SYM-NEXT:      SectionLen: 4
 ; SYM-NEXT:      ParameterHashIndex: 0x0
 ; SYM-NEXT:      TypeChkSectNum: 0x0
 ; SYM-NEXT:      SymbolAlignmentLog2: 2
 ; SYM-NEXT:      SymbolType: XTY_SD (0x1)
 ; SYM-NEXT:      StorageMappingClass: XMC_TC (0x3)
 ; SYM-NEXT:      StabInfoIndex: 0x0
 ; SYM-NEXT:      StabSectNum: 0x0
 ; SYM-NEXT:    }
 ; SYM-NEXT:  }
 ; SYM-NEXT:  Symbol {
 ; SYM-NEXT:    Index: [[#INDX+6]]
 ; SYM-NEXT:    Name: c
-; SYM-NEXT:    Value (RelocatableAddress): 0x94
+; SYM-NEXT:    Value (RelocatableAddress): 0xB0
 ; SYM-NEXT:    Section: .data
 ; SYM-NEXT:    Type: 0x0
 ; SYM-NEXT:    StorageClass: C_HIDEXT (0x6B)
 ; SYM-NEXT:    NumberOfAuxEntries: 1
 ; SYM-NEXT:    CSECT Auxiliary Entry {
 ; SYM-NEXT:      Index: [[#INDX+7]]
 ; SYM-NEXT:      SectionLen: 4
 ; SYM-NEXT:      ParameterHashIndex: 0x0
 ; SYM-NEXT:      TypeChkSectNum: 0x0
 ; SYM-NEXT:      SymbolAlignmentLog2: 2
 ; SYM-NEXT:      SymbolType: XTY_SD (0x1)
 ; SYM-NEXT:      StorageMappingClass: XMC_TC (0x3)
 ; SYM-NEXT:      StabInfoIndex: 0x0
 ; SYM-NEXT:      StabSectNum: 0x0
 ; SYM-NEXT:    }
 ; SYM-NEXT:  }
 ; SYM-NEXT:  Symbol {
 ; SYM-NEXT:    Index: [[#INDX+8]]
 ; SYM-NEXT:    Name: globa
-; SYM-NEXT:    Value (RelocatableAddress): 0x98
+; SYM-NEXT:    Value (RelocatableAddress): 0xB4
 ; SYM-NEXT:    Section: .data
 ; SYM-NEXT:    Type: 0x0
 ; SYM-NEXT:    StorageClass: C_HIDEXT (0x6B)
 ; SYM-NEXT:    NumberOfAuxEntries: 1
 ; SYM-NEXT:    CSECT Auxiliary Entry {
 ; SYM-NEXT:      Index: [[#INDX+9]]
 ; SYM-NEXT:      SectionLen: 4
 ; SYM-NEXT:      ParameterHashIndex: 0x0
 ; SYM-NEXT:      TypeChkSectNum: 0x0
 ; SYM-NEXT:      SymbolAlignmentLog2: 2
 ; SYM-NEXT:      SymbolType: XTY_SD (0x1)
 ; SYM-NEXT:      StorageMappingClass: XMC_TC (0x3)
 ; SYM-NEXT:      StabInfoIndex: 0x0
 ; SYM-NEXT:      StabSectNum: 0x0
 ; SYM-NEXT:    }
 ; SYM-NEXT:  }
 ; SYM-NEXT:  Symbol {
 ; SYM-NEXT:    Index: [[#INDX+10]]
 ; SYM-NEXT:    Name: ptr
-; SYM-NEXT:    Value (RelocatableAddress): 0x9C
+; SYM-NEXT:    Value (RelocatableAddress): 0xB8
 ; SYM-NEXT:    Section: .data
 ; SYM-NEXT:    Type: 0x0
 ; SYM-NEXT:    StorageClass: C_HIDEXT (0x6B)
 ; SYM-NEXT:    NumberOfAuxEntries: 1
 ; SYM-NEXT:    CSECT Auxiliary Entry {
 ; SYM-NEXT:      Index: [[#INDX+11]]
 ; SYM-NEXT:      SectionLen: 4
 ; SYM-NEXT:      ParameterHashIndex: 0x0
 ; SYM-NEXT:      TypeChkSectNum: 0x0
 ; SYM-NEXT:      SymbolAlignmentLog2: 2
 ; SYM-NEXT:      SymbolType: XTY_SD (0x1)
 ; SYM-NEXT:      StorageMappingClass: XMC_TC (0x3)
 ; SYM-NEXT:      StabInfoIndex: 0x0
 ; SYM-NEXT:      StabSectNum: 0x0
 ; SYM-NEXT:    }
 ; SYM-NEXT:  }
 ; SYM-NEXT:  Symbol {
 ; SYM-NEXT:    Index: [[#INDX+12]]
 ; SYM-NEXT:    Name: bar
-; SYM-NEXT:    Value (RelocatableAddress): 0xA0
+; SYM-NEXT:    Value (RelocatableAddress): 0xBC
 ; SYM-NEXT:    Section: .data
 ; SYM-NEXT:    Type: 0x0
 ; SYM-NEXT:    StorageClass: C_HIDEXT (0x6B)
 ; SYM-NEXT:    NumberOfAuxEntries: 1
 ; SYM-NEXT:    CSECT Auxiliary Entry {
 ; SYM-NEXT:      Index: [[#INDX+13]]
 ; SYM-NEXT:      SectionLen: 4
 ; SYM-NEXT:      ParameterHashIndex: 0x0
 ; SYM-NEXT:      TypeChkSectNum: 0x0
 ; SYM-NEXT:      SymbolAlignmentLog2: 2
 ; SYM-NEXT:      SymbolType: XTY_SD (0x1)
 ; SYM-NEXT:      StorageMappingClass: XMC_TC (0x3)
 ; SYM-NEXT:      StabInfoIndex: 0x0
 ; SYM-NEXT:      StabSectNum: 0x0
 ; SYM-NEXT:    }
 ; SYM-NEXT:  }
 ; SYM-NEXT:  Symbol {
 ; SYM-NEXT:    Index: [[#INDX+14]]
 ; SYM-NEXT:    Name: foo
-; SYM-NEXT:    Value (RelocatableAddress): 0xA4
+; SYM-NEXT:    Value (RelocatableAddress): 0xC0
 ; SYM-NEXT:    Section: .data
 ; SYM-NEXT:    Type: 0x0
 ; SYM-NEXT:    StorageClass: C_HIDEXT (0x6B)
 ; SYM-NEXT:    NumberOfAuxEntries: 1
 ; SYM-NEXT:    CSECT Auxiliary Entry {
 ; SYM-NEXT:      Index: [[#INDX+15]]
 ; SYM-NEXT:      SectionLen: 4
 ; SYM-NEXT:      ParameterHashIndex: 0x0
 ; SYM-NEXT:      TypeChkSectNum: 0x0
 ; SYM-NEXT:      SymbolAlignmentLog2: 2
 ; SYM-NEXT:      SymbolType: XTY_SD (0x1)
 ; SYM-NEXT:      StorageMappingClass: XMC_TC (0x3)
 ; SYM-NEXT:      StabInfoIndex: 0x0
 ; SYM-NEXT:      StabSectNum: 0x0
 ; SYM-NEXT:    }
 ; SYM-NEXT:  }
+; SYM-NEXT:  Symbol {
+; SYM-NEXT:    Index: [[#INDX+16]]
+; SYM-NEXT:    Name: foobar
+; SYM-NEXT:    Value (RelocatableAddress): 0xC4
+; SYM-NEXT:    Section: .data
+; SYM-NEXT:    Type: 0x0
+; SYM-NEXT:    StorageClass: C_HIDEXT (0x6B)
+; SYM-NEXT:    NumberOfAuxEntries: 1
+; SYM-NEXT:    CSECT Auxiliary Entry {
+; SYM-NEXT:      Index: [[#INDX+17]]
+; SYM-NEXT:      SectionLen: 4
+; SYM-NEXT:      ParameterHashIndex: 0x0
+; SYM-NEXT:      TypeChkSectNum: 0x0
+; SYM-NEXT:      SymbolAlignmentLog2: 2
+; SYM-NEXT:      SymbolType: XTY_SD (0x1)
+; SYM-NEXT:      StorageMappingClass: XMC_TC (0x3)
+; SYM-NEXT:      StabInfoIndex: 0x0
+; SYM-NEXT:      StabSectNum: 0x0
+; SYM-NEXT:    }
+; SYM-NEXT:  }