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

[PowerPC] 32-bit large code-model support for toc-data #85129

Merged
merged 4 commits into from
Apr 17, 2024

Conversation

syzaara
Copy link
Contributor

@syzaara syzaara commented Mar 13, 2024

This patch adds the pseudo op ADDItocL for 32-bit large code-model support for toc-data.

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 13, 2024

@llvm/pr-subscribers-backend-powerpc

Author: Zaara Syeda (syzaara)

Changes

This patch adds the pseudo op ADDItocL for 32-bit large code-model support for toc-data.


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

6 Files Affected:

  • (modified) llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp (+46-9)
  • (modified) llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp (+25-3)
  • (modified) llvm/lib/Target/PowerPC/PPCInstrInfo.cpp (+1)
  • (modified) llvm/lib/Target/PowerPC/PPCInstrInfo.td (+3-1)
  • (modified) llvm/lib/Target/PowerPC/PPCSubtarget.cpp (+5)
  • (modified) llvm/test/CodeGen/PowerPC/toc-data.ll (+65)
diff --git a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
index 542854ec9b99fd..852f48f895eb5d 100644
--- a/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ b/llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -1117,15 +1117,28 @@ void PPCAsmPrinter::emitInstruction(const MachineInstr *MI) {
 
     MCSymbolRefExpr::VariantKind VK = GetVKForMO(MO);
 
-    // Always use TOC on AIX. Map the global address operand to be a reference
-    // to the TOC entry we will synthesize later. 'TOCEntry' is a label used to
-    // reference the storage allocated in the TOC which contains the address of
-    // 'MOSymbol'.
-    MCSymbol *TOCEntry =
-        lookUpOrCreateTOCEntry(MOSymbol, getTOCEntryTypeForMO(MO), VK);
-    const MCExpr *Exp = MCSymbolRefExpr::create(TOCEntry,
-                                                MCSymbolRefExpr::VK_PPC_U,
-                                                OutContext);
+    // If the symbol isn't toc-data then use the TOC on AIX.
+    // Map the global address operand to be a reference to the TOC entry we
+    // will synthesize later. 'TOCEntry' is a label used to reference the
+    // storage allocated in the TOC which contains the address of 'MOSymbol'.
+    // If the toc-data attribute is used, the TOC entry contains the data
+    // rather than the address of the MOSymbol.
+    auto isSymbolTD = [](const MachineOperand &MO) {
+      if (!MO.isGlobal())
+        return false;
+
+      const GlobalVariable *GV = dyn_cast<GlobalVariable>(MO.getGlobal());
+      if (!GV)
+        return false;
+
+      return GV->hasAttribute("toc-data");
+    };
+
+    if (!isSymbolTD(MO))
+      MOSymbol = lookUpOrCreateTOCEntry(MOSymbol, getTOCEntryTypeForMO(MO), VK);
+
+    const MCExpr *Exp = MCSymbolRefExpr::create(
+        MOSymbol, MCSymbolRefExpr::VK_PPC_U, OutContext);
     TmpInst.getOperand(2) = MCOperand::createExpr(Exp);
     EmitToStreamer(*OutStreamer, TmpInst);
     return;
@@ -1236,6 +1249,30 @@ void PPCAsmPrinter::emitInstruction(const MachineInstr *MI) {
     EmitToStreamer(*OutStreamer, TmpInst);
     return;
   }
+  case PPC::ADDItocL: {
+    // Transform %xd = ADDItocL %xs, @sym
+    LowerPPCMachineInstrToMCInst(MI, TmpInst, *this);
+
+    // Change the opcode to load address.
+    TmpInst.setOpcode(PPC::LA);
+
+    const MachineOperand &MO = MI->getOperand(2);
+    assert(MO.isGlobal() && "Invalid operand for ADDItocL.");
+
+    LLVM_DEBUG(assert(
+        !(MO.isGlobal() && Subtarget->isGVIndirectSymbol(MO.getGlobal())) &&
+        "Interposable definitions must use indirect access."));
+
+    // Map the operand to its corresponding MCSymbol.
+    const MCSymbol *const MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
+
+    const MCExpr *Exp = MCSymbolRefExpr::create(
+        MOSymbol, MCSymbolRefExpr::VK_PPC_L, OutContext);
+
+    TmpInst.getOperand(2) = MCOperand::createExpr(Exp);
+    EmitToStreamer(*OutStreamer, TmpInst);
+    return;
+  }
   case PPC::ADDItocL8: {
     // Transform %xd = ADDItocL8 %xs, @sym
     LowerPPCMachineInstrToMCInst(MI, TmpInst, *this);
diff --git a/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp b/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
index 0c25accd1d6ce6..35cdb75760a063 100644
--- a/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
@@ -6125,9 +6125,10 @@ void PPCDAGToDAGISel::Select(SDNode *N) {
            " ELF/AIX or 32-bit AIX in the following.");
 
     // Transforms the ISD::TOC_ENTRY node for 32-bit AIX large code model mode
-    // or 64-bit medium (ELF-only) or large (ELF and AIX) code model code. We
-    // generate two instructions as described below. The first source operand
-    // is a symbol reference. If it must be toc-referenced according to
+    // or 64-bit medium (ELF-only) or large (ELF and AIX) code model code non
+    // toc-data symbols.
+    // We generate two instructions as described below. The first source
+    // operand is a symbol reference. If it must be toc-referenced according to
     // Subtarget, we generate:
     // [32-bit AIX]
     //   LWZtocL(@sym, ADDIStocHA(%r2, @sym))
@@ -6135,6 +6136,13 @@ void PPCDAGToDAGISel::Select(SDNode *N) {
     //   LDtocL(@sym, ADDIStocHA8(%x2, @sym))
     // Otherwise we generate:
     //   ADDItocL8(ADDIStocHA8(%x2, @sym), @sym)
+
+    // For large code model toc-data symbols we generate:
+    // [32-bit AIX]
+    //   ADDItocL(ADDIStocHA(%x2, @sym), @sym)
+    // [64-bit AIX]
+    //   ADDItocL8(ADDIStocHA8(%x2, @sym), @sym)
+
     SDValue GA = N->getOperand(0);
     SDValue TOCbase = N->getOperand(1);
 
@@ -6142,6 +6150,20 @@ void PPCDAGToDAGISel::Select(SDNode *N) {
     SDNode *Tmp = CurDAG->getMachineNode(
         isPPC64 ? PPC::ADDIStocHA8 : PPC::ADDIStocHA, dl, VT, TOCbase, GA);
 
+    // On AIX if the symbol has the toc-data attribute it will be defined
+    // in the TOC entry, so we use an ADDItocL similar to the medium code
+    // model ELF abi.
+    if (isAIXABI &&
+        hasTocDataAttr(GA, CurDAG->getDataLayout().getPointerSize())) {
+      if (isPPC64)
+        report_fatal_error(
+            "64-bit large code model toc-data not yet supported");
+
+      ReplaceNode(N, CurDAG->getMachineNode(PPC::ADDItocL, dl, VT,
+                                            SDValue(Tmp, 0), GA));
+      return;
+    }
+
     if (PPCLowering->isAccessedAsGotIndirect(GA)) {
       // If it is accessed as got-indirect, we need an extra LWZ/LD to load
       // the address.
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
index 5f5eb31a5a85fa..e8410a5c45c761 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
@@ -1077,6 +1077,7 @@ bool PPCInstrInfo::isReallyTriviallyReMaterializable(
   case PPC::LIS8:
   case PPC::ADDIStocHA:
   case PPC::ADDIStocHA8:
+  case PPC::ADDItocL:
   case PPC::ADDItocL8:
   case PPC::LOAD_STACK_GUARD:
   case PPC::PPCLdFixedAddr:
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.td b/llvm/lib/Target/PowerPC/PPCInstrInfo.td
index 82da1a3c305983..776213889abdf1 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.td
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.td
@@ -3346,11 +3346,13 @@ def ADDIStocHA : PPCEmitTimePseudo<(outs gprc:$rD), (ins gprc_nor0:$reg, tocentr
                        "#ADDIStocHA",
                        [(set i32:$rD,
                          (PPCtoc_entry i32:$reg, tglobaladdr:$disp))]>;
-// Local Data Transform
+// TOC Data Transform AIX
 def ADDItoc : PPCEmitTimePseudo<(outs gprc:$rD), (ins tocentry32:$disp, gprc:$reg),
                    "#ADDItoc",
                    [(set i32:$rD,
                      (PPCtoc_entry tglobaladdr:$disp, i32:$reg))]>;
+def ADDItocL : PPCEmitTimePseudo<(outs gprc:$rD), (ins gprc_nor0:$reg, tocentry32:$disp),
+                   "#ADDItocL", []>;
 
 // Get Global (GOT) Base Register offset, from the word immediately preceding
 // the function label.
diff --git a/llvm/lib/Target/PowerPC/PPCSubtarget.cpp b/llvm/lib/Target/PowerPC/PPCSubtarget.cpp
index 884f2f5c57b258..f0f736e39de0c7 100644
--- a/llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ b/llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -208,6 +208,11 @@ void PPCSubtarget::tocDataChecks(unsigned PointerSize,
 }
 
 bool PPCSubtarget::isGVIndirectSymbol(const GlobalValue *GV) const {
+  // toc-data attribute means skip the indirection we usually use on AIX
+  if (isAIXABI())
+    if (const GlobalVariable *GVar = dyn_cast<GlobalVariable>(GV))
+      if (GVar->hasAttribute("toc-data"))
+        return false;
   // Large code model always uses the TOC even for local symbols.
   if (TM.getCodeModel() == CodeModel::Large)
     return true;
diff --git a/llvm/test/CodeGen/PowerPC/toc-data.ll b/llvm/test/CodeGen/PowerPC/toc-data.ll
index cbf3be9fcaad05..f3b74e54fb8d09 100644
--- a/llvm/test/CodeGen/PowerPC/toc-data.ll
+++ b/llvm/test/CodeGen/PowerPC/toc-data.ll
@@ -12,6 +12,14 @@
 ; RUN: llc -mtriple powerpc-ibm-aix-xcoff -verify-machineinstrs -O0 < %s | FileCheck %s --check-prefix TEST32
 ; RUN: llc -mtriple powerpc64-ibm-aix-xcoff -verify-machineinstrs -O0 < %s | FileCheck %s --check-prefix TEST64
 
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff -code-model=large -verify-machineinstrs < %s \
+; RUN:     -stop-before=ppc-vsx-copy | FileCheck %s --check-prefix CHECK32LARGE
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff -code-model=large -verify-machineinstrs < %s | FileCheck %s --check-prefix TEST32LARGE
+
+; Global variables i and f have the toc-data attribute.
+; In the following functions, those writing to or reading from
+; variables i and f should use the toc-data access pattern.
+; All remaining variables should use the regular toc access sequence.
 @i = dso_local global i32 0, align 4 #0
 @d = dso_local local_unnamed_addr global double 3.141590e+00, align 8
 @f = dso_local local_unnamed_addr global float 0x4005BE76C0000000, align 4 #0
@@ -44,6 +52,15 @@ define dso_local void @write_int(i32 signext %in) {
 ; TEST64:           la 4, i[TD](2)
 ; TEST64-NEXT:      stw 3, 0(4)
 
+; CHECK32LARGE: name:            write_int
+; CHECK32LARGE:      %[[SCRATCH1:[0-9]+]]:gprc_and_gprc_nor0 = ADDIStocHA $r2, @i
+; CHECK32LARGE-NEXT: %[[SCRATCH2:[0-9]+]]:gprc_and_gprc_nor0 = ADDItocL killed %[[SCRATCH1]], @i
+; CHECK32LARGE-NEXT: STW %{{[0-9]+}}, 0, killed %[[SCRATCH2]] :: (store (s32) into @i)
+
+; TEST32LARGE:         .write_int:
+; TEST32LARGE:          addis 4, i[TD]@u(2)
+; TEST32LARGE-NEXT:	la 4, i[TD]@l(4)
+; TEST32LARGE-NEXT:	stw 3, 0(4)
 
 define dso_local i64 @read_ll() {
   entry:
@@ -70,6 +87,15 @@ define dso_local i64 @read_ll() {
 ; TEST64:         ld 3, L..C0(2)
 ; TEST64-NEXT:    ld 3, 0(3)
 
+; CHECK32LARGE: name:            read_ll
+; CHECK32LARGE: %[[SCRATCH1:[0-9]+]]:gprc_and_gprc_nor0 = ADDIStocHA $r2, @ll
+; CHECK32LARGE: LWZtocL @ll, killed %[[SCRATCH1]] :: (load (s32) from got)
+
+; TEST32LARGE:         .read_ll:
+; TEST32LARGE:          addis 3, L..C0@u(2)
+; TEST32LARGE-NEXT:	lwz 4, L..C0@l(3)
+; TEST32LARGE-NEXT:	lwz 3, 0(4)
+; TEST32LARGE-NEXT:	lwz 4, 4(4)
 
 define dso_local float @read_float() {
   entry:
@@ -96,6 +122,15 @@ define dso_local float @read_float() {
 ; TEST64:         la 3, f[TD](2)
 ; TEST64-NEXT:    lfs 1, 0(3)
 
+; CHECK32LARGE: name:            read_float
+; CHECK32LARGE:      %[[SCRATCH1:[0-9]+]]:gprc_and_gprc_nor0 = ADDIStocHA $r2, @f
+; CHECK32LARGE-NEXT: %[[SCRATCH2:[0-9]+]]:gprc_and_gprc_nor0 = ADDItocL killed %[[SCRATCH1]], @f
+; CHECK32LARGE-NEXT: LFS 0, killed %[[SCRATCH2]] :: (dereferenceable load (s32) from @f)
+
+; TEST32LARGE:         .read_float:
+; TEST32LARGE:          addis 3, f[TD]@u(2)
+; TEST32LARGE-NEXT:	la 3, f[TD]@l(3)
+; TEST32LARGE-NEXT:	lfs 1, 0(3)
 
 define dso_local void @write_double(double %in) {
   entry:
@@ -121,6 +156,14 @@ define dso_local void @write_double(double %in) {
 ; TEST64:         ld 3, L..C1(2)
 ; TEST64-NEXT:    stfd 1, 0(3)
 
+; CHECK32LARGE: name:            write_double
+; CHECK32LARGE: %[[SCRATCH1:[0-9]+]]:gprc_and_gprc_nor0 = ADDIStocHA $r2, @d
+; CHECK32LARGE: LWZtocL @d, killed %[[SCRATCH1]] :: (load (s32) from got)
+
+; TEST32LARGE:         .write_double:
+; TEST32LARGE:          addis 3, L..C1@u(2)
+; TEST32LARGE-NEXT:	lwz 3, L..C1@l(3)
+; TEST32LARGE-NEXT:	stfd 1, 0(3)
 
 define dso_local nonnull ptr @addr() {
   entry:
@@ -144,6 +187,15 @@ define dso_local nonnull ptr @addr() {
 ; TEST64:       .addr
 ; TEST64:         la 3, i[TD](2)
 
+; CHECK32LARGE: name:            addr
+; CHECK32LARGE:      %[[SCRATCH1:[0-9]+]]:gprc_and_gprc_nor0 = ADDIStocHA $r2, @i
+; CHECK32LARGE-NEXT: %[[SCRATCH2:[0-9]+]]:gprc = ADDItocL killed %[[SCRATCH1]], @i
+; CHECK32LARGE-NEXT: $r3 = COPY %[[SCRATCH2]]
+
+; TEST32LARGE:         .addr:
+; TEST32LARGE:          addis 3, i[TD]@u(2)
+; TEST32LARGE-NEXT:	la 3, i[TD]@l(3)
+
 ; TEST32:         .toc
 ; TEST32:           .tc ll[TC],ll[RW]
 ; TEST32-NOT:       .csect ll[TD]
@@ -170,4 +222,17 @@ define dso_local nonnull ptr @addr() {
 ; TEST64-NEXT:      .globl f[TD]
 ; TEST64-NOT:       .tc f[TD],f[RW]
 
+; TEST32LARGE:         .toc
+; TEST32LARGE:           .tc ll[TE],ll[RW]
+; TEST32LARGE-NOT:       .csect ll[TD]
+; TEST32LARGE:           .tc d[TE],d[RW]
+; TEST32LARGE-NOT:       .csect d[TD],2
+; TEST32LARGE:           .csect i[TD],2
+; TEST32LARGE-NEXT:      .globl  i[TD]
+; TEST32LARGE-NEXT:      .align  2
+; TEST32LARGE-NOT:       .tc i[TE],i[RW]
+; TEST32LARGE:           .csect f[TD],2
+; TEST32LARGE-NEXT:      .globl f[TD]
+; TEST32LARGE-NOT:       .tc f[TE],f[RW]
+
 attributes #0 = { "toc-data" }


LLVM_DEBUG(assert(
!(MO.isGlobal() && Subtarget->isGVIndirectSymbol(MO.getGlobal())) &&
"Interposable definitions must use indirect access."));
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
"Interposable definitions must use indirect access."));
"Interposable definitions must use indirect accesses."));

// in the TOC entry, so we use an ADDItocL similar to the medium code
// model ELF abi.
if (isAIXABI &&
hasTocDataAttr(GA, CurDAG->getDataLayout().getPointerSize())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I am mistaken, but it doesn't look like hasTocDataAttr() uses CurDAG->getDataLayout().getPointerSize(). Do we need the second parameter?

@syzaara syzaara force-pushed the syzaara/32BitPicLargeTocData branch from 77afc7b to 13daabc Compare March 18, 2024 21:39

; TEST32LARGE: .write_int:
; TEST32LARGE: addis 4, i[TD]@u(2)
; TEST32LARGE-NEXT: la 4, i[TD]@l(4)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not related to this patch, here we should have a peephole opportunity that we can add the lower part relocation @l to the consuming stw?

Can we add a fixme here? We are trying to improve the code generation for toc-data at small code model in #76488 . But seems we also have the opportunity for the large model.

; TEST32LARGE: .read_float:
; TEST32LARGE: addis 3, f[TD]@u(2)
; TEST32LARGE-NEXT: la 3, f[TD]@l(3)
; TEST32LARGE-NEXT: lfs 1, 0(3)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, the la should be able to merge into the latter lfs?

def ADDItoc : PPCEmitTimePseudo<(outs gprc:$rD), (ins tocentry32:$disp, gprc:$reg),
"#ADDItoc",
[(set i32:$rD,
(PPCtoc_entry tglobaladdr:$disp, i32:$reg))]>;
def ADDItocL : PPCEmitTimePseudo<(outs gprc:$rD), (ins gprc_nor0:$reg, tocentry32:$disp),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, you defined a new pseudo and it will be lowered to the real instruction addi. Should we add this new instruction into

  • scheduling model, like llvm/lib/Target/PowerPC/P10InstrResources.td. (P9/P8 scheduling model's regular expression is able to cover this. P7 is bad, it also does not cover 64bit version, maybe we can just leave them for now.)
  • llvm/lib/Target/PowerPC/PPCMacroFusion.def
  • Peephole optimization in getForwardingDefMI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added it to P10InstrResources.td and PPCMacroFusion.def.
getForwardingDefMI seems more involved and requires some investigation, so I'm thinking to leave that for a follow up patch.

@@ -1272,6 +1285,30 @@ void PPCAsmPrinter::emitInstruction(const MachineInstr *MI) {
EmitToStreamer(*OutStreamer, TmpInst);
return;
}
case PPC::ADDItocL: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

format related:
PPC::ADDItocL and PPC::ADDItocL8 are almost the same. Is it possible to merge them into one case?

return GV->hasAttribute("toc-data");
};

if (!isSymbolTD(MO))
Copy link
Collaborator

Choose a reason for hiding this comment

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

format related:
since the lambda function isSymbolTD is only used once, can we use anonymous lambda function in the if body? Like:

if ([](const MachineOperand &MO){}(MO)) {
}

Zaara Syeda added 2 commits April 8, 2024 10:46
This patch adds the pseudo op ADDItocL for 32-bit large code-model
support for toc-data.
@syzaara syzaara force-pushed the syzaara/32BitPicLargeTocData branch from 13daabc to 257bf03 Compare April 8, 2024 14:57
SDValue(Tmp, 0), GA));
return;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

At line 6195, for 32-bit, should we use ADDItocL instead of ADDItocL8? Although seems there is no 32-bit case reaching that logic.(It uses MVT::i64 at this phase, on 32-bit it should fail?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Line 6195 is handling the Linux medium code model path. For tocdata 64-bit with large code model, I will have a follow up patch to use ADDItocL8 (with the appropriate enhancements made in PPCAsmPrinter.cpp). ADDItocL8 will then be used on line 6175 above.
I plan to post the 64-bit support next once this 32-bit patch is approved and merged.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK. Thanks for explanation.

if (Op == PPC::ADDItocL8)
assert((MO.isGlobal() || MO.isCPI()) && "Invalid operand for ADDItocL8.");
else
assert(MO.isGlobal() && "Invalid operand for ADDItocL.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks weird to me. If ADDItocL8 can be used for constant pool entries at 64-bit, why ADDItocL can not be used for contant pool entries at 32-bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ADDItocL is added specifically for toc-data on AIX. ADDItocL8 is already used on Linux medium code model for TOC access, so its possible that the MO is a CPI.
ADDItocL however is only for toc-data symbols which must be globals.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ADDItocL8 is already used on Linux medium code model for TOC access, so its possible that the MO is a CPI.

ADDItocL8 is used with CPI MO in global-isel when lowering a constant pool entry. But that is only for PPC64LE.

I originally thought ADDItocL should be used there for 32 bit. So maybe it is not strange that we use ADDItocL for CPI MO too. IIUC in the patch 34627e3 where ADDItocL was firstly introduced, there is no CPI MO usage for it.
But seems all 32-bit ABI(ELF32 and AIX32) does not allow direct access to TOC, so we should not use ADDItocL for 32bit, we should use LWZtocL.

I agree with you. We can limit ADDItocL to global address.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Code format:

assert((Op == PPC::ADDItocL8) ? (MO.isGlobal() || MO.isCPI()) : MO.isGlobal() && "Invalid operand for ADDItocL8.");

Copy link
Collaborator

@chenzheng1030 chenzheng1030 left a comment

Choose a reason for hiding this comment

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

LGTM. I don't have further comments except one nit.

SDValue(Tmp, 0), GA));
return;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK. Thanks for explanation.

@syzaara syzaara merged commit 76ad289 into llvm:main Apr 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants