Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AIX] [XCOFF] Add support for common and local common symbols in the TOC #79530

Merged
merged 4 commits into from
Jan 31, 2024

Conversation

syzaara
Copy link
Contributor

@syzaara syzaara commented Jan 26, 2024

This patch adds support for common and local symbols in the TOC for AIX. Note that we need to update isVirtualSection so as a common symbol in TOC will have the symbol type XTY_CM and will be initialized when placed in the TOC so sections with this type are no longer virtual.

@llvmbot llvmbot added the mc Machine (object) code label Jan 26, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 26, 2024

@llvm/pr-subscribers-mc

Author: Zaara Syeda (syzaara)

Changes

This patch adds support for common and local symbols in the TOC for AIX. Note that we need to update isVirtualSection so as a common symbol in TOC will have the symbol type XTY_CM and will be initialized when placed in the TOC so sections with this type are no longer virtual.


Full diff: https://github.com/llvm/llvm-project/pull/79530.diff

5 Files Affected:

  • (modified) llvm/lib/MC/MCSectionXCOFF.cpp (+8-1)
  • (modified) llvm/lib/MC/XCOFFObjectWriter.cpp (+8-2)
  • (modified) llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp (+19-4)
  • (modified) llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp (+2-5)
  • (modified) llvm/test/CodeGen/PowerPC/basic-toc-data-local-linkage.ll (+6-4)
diff --git a/llvm/lib/MC/MCSectionXCOFF.cpp b/llvm/lib/MC/MCSectionXCOFF.cpp
index 02b1b972f339bf0..95d32e3580e3f55 100644
--- a/llvm/lib/MC/MCSectionXCOFF.cpp
+++ b/llvm/lib/MC/MCSectionXCOFF.cpp
@@ -82,6 +82,11 @@ void MCSectionXCOFF::printSwitchToSection(const MCAsmInfo &MAI, const Triple &T,
   }
 
   if (isCsect() && getMappingClass() == XCOFF::XMC_TD) {
+    // Common csect type (uninitialized storage) does not have to print csect
+    // directive for section switching unless it is local.
+    if (getKind().isCommon() && !getKind().isBSSLocal())
+      return;
+
     assert((getKind().isBSSExtern() || getKind().isBSSLocal()) &&
            "Unexepected section kind for toc-data");
     printCsectDirective(OS);
@@ -135,5 +140,7 @@ bool MCSectionXCOFF::isVirtualSection() const {
     return false;
   assert(isCsect() &&
          "Handling for isVirtualSection not implemented for this section!");
-  return XCOFF::XTY_CM == CsectProp->Type;
+  // XTY_CM sections are virtual except for toc-data symbols.
+  return (XCOFF::XTY_CM == CsectProp->Type) &&
+         (getMappingClass() != XCOFF::XMC_TD);
 }
diff --git a/llvm/lib/MC/XCOFFObjectWriter.cpp b/llvm/lib/MC/XCOFFObjectWriter.cpp
index 343e2fc877bc3b1..11f8a72edeadafc 100644
--- a/llvm/lib/MC/XCOFFObjectWriter.cpp
+++ b/llvm/lib/MC/XCOFFObjectWriter.cpp
@@ -533,9 +533,15 @@ CsectGroup &XCOFFObjectWriter::getCsectGroup(const MCSectionXCOFF *MCSec) {
     return TOCCsects;
   case XCOFF::XMC_TC:
   case XCOFF::XMC_TE:
-  case XCOFF::XMC_TD:
     assert(XCOFF::XTY_SD == MCSec->getCSectType() &&
-           "Only an initialized csect can contain TC entry.");
+           "A TOC symbol must be an initialized csect.");
+    assert(!TOCCsects.empty() &&
+           "We should at least have a TOC-base in this CsectGroup.");
+    return TOCCsects;
+  case XCOFF::XMC_TD:
+    assert((XCOFF::XTY_SD == MCSec->getCSectType() ||
+            XCOFF::XTY_CM == MCSec->getCSectType()) &&
+           "Symbol type incompatible with toc-data.");
     assert(!TOCCsects.empty() &&
            "We should at least have a TOC-base in this CsectGroup.");
     return TOCCsects;
diff --git a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
index 4f15ba497d84c45..1f4476d3907cdd3 100644
--- a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -2572,12 +2572,18 @@ void PPCAIXAsmPrinter::emitGlobalVariableHelper(const GlobalVariable *GV) {
     GVSym->setStorageClass(
         TargetLoweringObjectFileXCOFF::getStorageClassForGlobal(GV));
 
-    if (GVKind.isBSSLocal() || GVKind.isThreadBSSLocal())
+    if (GVKind.isBSSLocal() && Csect->getMappingClass() == XCOFF::XMC_TD) {
+      OutStreamer->emitZeros(Size);
+    } else if (GVKind.isBSSLocal() || GVKind.isThreadBSSLocal()) {
+      assert(Csect->getMappingClass() != XCOFF::XMC_TD &&
+             "BSS local toc-data already handled and TLS variables "
+             "incompatible with XMC_TD");
       OutStreamer->emitXCOFFLocalCommonSymbol(
           OutContext.getOrCreateSymbol(GVSym->getSymbolTableName()), Size,
           GVSym, Alignment);
-    else
+    } else {
       OutStreamer->emitCommonSymbol(GVSym, Size, Alignment);
+    }
     return;
   }
 
@@ -2738,8 +2744,17 @@ void PPCAIXAsmPrinter::emitEndOfAsmFile(Module &M) {
     TS->emitTCEntry(*I.first.first, I.first.second);
   }
 
-  for (const auto *GV : TOCDataGlobalVars)
-    emitGlobalVariableHelper(GV);
+  // Traverse the list of global variables twice, emitting all of the
+  // non-common global variables before the common ones, as emitting a
+  // .comm directive changes the scope from .toc to the common symbol.
+  for (const auto *GV : TOCDataGlobalVars) {
+    if (!GV->hasCommonLinkage())
+      emitGlobalVariableHelper(GV);
+  }
+  for (const auto *GV : TOCDataGlobalVars) {
+    if (GV->hasCommonLinkage())
+      emitGlobalVariableHelper(GV);
+  }
 }
 
 bool PPCAIXAsmPrinter::doInitialization(Module &M) {
diff --git a/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp b/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
index b57d185bb638b8c..b86c9abeecf35f8 100644
--- a/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
@@ -551,13 +551,10 @@ static bool hasTocDataAttr(SDValue Val, unsigned PointerSize) {
          "A GlobalVariable with size larger than a TOC entry is not currently "
          "supported by the toc data transformation.");
 
-  if (GV->hasLocalLinkage() || GV->hasPrivateLinkage())
-    report_fatal_error("A GlobalVariable with private or local linkage is not "
+  if (GV->hasPrivateLinkage())
+    report_fatal_error("A GlobalVariable with private linkage is not "
                        "currently supported by the toc data transformation.");
 
-  assert(!GV->hasCommonLinkage() &&
-         "Tentative definitions cannot have the mapping class XMC_TD.");
-
   return true;
 }
 
diff --git a/llvm/test/CodeGen/PowerPC/basic-toc-data-local-linkage.ll b/llvm/test/CodeGen/PowerPC/basic-toc-data-local-linkage.ll
index 0e131672b421e4b..4352852533ffb31 100644
--- a/llvm/test/CodeGen/PowerPC/basic-toc-data-local-linkage.ll
+++ b/llvm/test/CodeGen/PowerPC/basic-toc-data-local-linkage.ll
@@ -1,6 +1,6 @@
-; RUN: not --crash llc  -mtriple powerpc-ibm-aix-xcoff  -verify-machineinstrs \
+; RUN: llc  -mtriple powerpc-ibm-aix-xcoff  -verify-machineinstrs \
 ; RUN:     < %s 2>&1 | FileCheck %s
-; RUN: not --crash llc  -mtriple powerpc64-ibm-aix-xcoff  -verify-machineinstrs \
+; RUN: llc  -mtriple powerpc64-ibm-aix-xcoff  -verify-machineinstrs \
 ; RUN:     < %s 2>&1 | FileCheck %s
 
 @ilocal = internal global i32 0, align 4 #0
@@ -11,6 +11,8 @@ define dso_local i32 @read_i32_local_linkage() {
     ret i32 %0
 }
 
-; CHECK: LLVM ERROR: A GlobalVariable with private or local linkage is not currently supported by the toc data transformation.
-
 attributes #0 = { "toc-data" }
+
+; CHECK:      .toc
+; CHECK-NEXT: .csect ilocal[TD],2
+; CHECK-NEXT: .space  4

Copy link

github-actions bot commented Jan 29, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@@ -0,0 +1,204 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple powerpc-ibm-aix-xcoff -verify-machineinstrs < %s | FileCheck %s --check-prefix CHECK
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit , use the check-prefix=CHECK as the same style as --check-prefix=OBJ32 in the following.

; CHECK64-NEXT: ld 5, L..C1(2) # @a3
; CHECK64-NEXT: stw 3, 0(4)
; CHECK64-NEXT: stw 3, 0(5)
; CHECK64-NEXT: blr
Copy link
Contributor

@diggerlin diggerlin Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the most assemble are same. we do not need duplicate here. please fix all the following duplication.
the only different is
lwz 5, L..C1(2) # @a3 vs ld 5, L..C1(2) # @a3
lwz 4, L..C1(2) # @a3 vs ld 4, L..C1(2) # @a3

you can try

llc -mtriple powerpc-ibm-aix-xcoff -verify-machineinstrs < %s | FileCheck %s -DINSTR=lwz --check-prefix=CHECK

and

llc -mtriple powerpc64-ibm-aix-xcoff -verify-machineinstrs < %s | FileCheck %s -DINSTR=ld --check-prefix=CHECK

; CHECK: # %bb.0: # %entry
; CHECK-NEXT: la 4, a2TD
; CHECK-NEXT: la 5, a1TD
; CHECK-NEXT: stw 3, 0(4)
; CHECK-NEXT: [[INSTR]] 4, L..C0(2) # @a4
; CHECK-NEXT: stw 3, 0(5)
; CHECK-NEXT: [[INSTR]] 5, L..C1(2) # @A3
; CHECK-NEXT: stw 3, 0(4)
; CHECK-NEXT: stw 3, 0(5)
; CHECK-NEXT: blr

@@ -0,0 +1,204 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple powerpc-ibm-aix-xcoff -verify-machineinstrs < %s | FileCheck %s --check-prefix CHECK
; RUN: llc -mtriple powerpc64-ibm-aix-xcoff -verify-machineinstrs < %s | FileCheck %s --check-prefix CHECK64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor

@diggerlin diggerlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Zaara Syeda added 4 commits January 31, 2024 10:00
This patch adds support for common and local symbols in the TOC for AIX.
Note that we need to update isVirtualSection so as a common symbol in TOC will
have the symbol type XTY_CM and will be initialized when placed in the TOC so
sections with this type are no longer virtual.
@syzaara syzaara merged commit a03a6e9 into llvm:main Jan 31, 2024
4 checks passed
carlosgalvezp pushed a commit to carlosgalvezp/llvm-project that referenced this pull request Feb 1, 2024
…TOC (llvm#79530)

This patch adds support for common and local symbols in the TOC for AIX.
Note that we need to update isVirtualSection so as a common symbol in
TOC will have the symbol type XTY_CM and will be initialized when placed
in the TOC so sections with this type are no longer virtual.

---------

Co-authored-by: Zaara Syeda <syzaara@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants