[PowerPC][XCOFF] Fix expansion of LWZtoc Pseudo for AIX.

[Xiangling_L]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

652

As you mentioned before, should we assert Darwin target is invalid for this toc-related pseudo? Something like:

assert (!isDarwin && "TOC is an ELF/XCOFF construct");

674

You seems miss a comment here , which matches the later "Otherwise" on line 684

// Create a reference to the GOT entry for the symbol. The GOT entry will be synthesized later.

695

Should we be more specific?
Like:

This pseudo should only be selected for 32-bit small code model.

llvm/test/CodeGen/PowerPC/lower-globaladdr32-aix-asm.ll

4

Personally, I prefer keep the testcase as minimal as possible, so maybe we can remove some attributes like:
"local_unnamed_addr",
"; Function Attrs: norecurse nounwind",
"#0",
"align 4" etc.

[sfertile]

• Added an assertion expressing the platform must not be Darwin.
• Added missing comment
• Fixed assertion message to be more specific
• Cleaned up the test.

[sfertile]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

652

Good idea. Added.

llvm/test/CodeGen/PowerPC/lower-globaladdr32-aix-asm.ll

4

Agreed. Cleaned up.

[Xiangling_L]

Just a minor nit:

clean up "local_unnamed_addr" in testcase? or even "dso_local"?

llvm/test/CodeGen/PowerPC/lower-globaladdr32-aix-asm.ll

8

Clean up this "local_unnamed_addr" as well?

[sfertile]

In D67853#1680848, @Xiangling_L wrote:

Just a minor nit:

clean up "local_unnamed_addr" in testcase? or even "dso_local"?

Done, and thanks for the review.

llvm/test/CodeGen/PowerPC/lower-globaladdr32-aix-asm.ll

8

I got sloppy and missed this one. I've cleaned up in the final commit.

[hubert.reinterpretcast]

llvm/trunk/lib/Target/PowerPC/PPCAsmPrinter.cpp

696 ↗

(On Diff #221563)

I would prefer if we mentioned "label" somewhere in the comment as the term that maps better in the context of AIX. The implicit property is also limited to certain contexts (the Assembler Language Reference describes the condition as use of the label as a displacement in a D-form instruction), and the comment is insufficiently qualified to make that clear.

[sfertile]

llvm/trunk/lib/Target/PowerPC/PPCAsmPrinter.cpp

696 ↗

(On Diff #221563)

I would prefer if we mentioned "label" somewhere in the comment as the term that maps better in the context of AIX.

Can you expand on that, I'm not sure what semantic difference there is between a local symbol and a local label. I considered them equivalent, but can be wrong.

The implicit property is also limited to certain contexts (the Assembler Language Reference describes the condition as use of the label as a displacement in a D-form instruction), and the comment is insufficiently qualified to make that clear.

What about:

// AIX uses the local symbol directly for the operand; that the symbol is
 // accessed toc-relative in a lwz instruction is implicit.

[hubert.reinterpretcast]

llvm/trunk/lib/Target/PowerPC/PPCAsmPrinter.cpp

696 ↗

(On Diff #221563)

Maybe: "[ ... ] directly as the lwz displacement operand for references into the toc section. The displacement value will be generated relative to the toc-base."

As for the difference between a label and a local symbol: A label in the assembly may be used to provide a definition for a symbol, but does not itself form a symbol. When using a reference to a label that is not a symbol, the generated relocation should refer to the symbol table entry of the csect. When using a reference to a label that is a symbol, the generated relocation refers to the symbol table entry of the symbol. For toc entries, the csect is the entry itself, so a non-symbol label to the start of the entry would act like the entry symbol/csect.

[sfertile]

Thanks for the explanation Hubert. I've updated the comments and committed.

[hubert.reinterpretcast]

In D67853#1684295, @sfertile wrote:

Thanks for the explanation Hubert. I've updated the comments and committed.

Great; thanks.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

if (Subtarget->isSecurePlt() && isPositionIndependent() ) {
       TmpInst.getOperand(0) = PICR;
       TmpInst.getOperand(1) = TR;
       TmpInst.getOperand(2) = PICR;
       EmitToStreamer(*OutStreamer, TmpInst);
       return;
     }
   }
   case PPC::LWZtoc: {
-    // Transform %r3 = LWZtoc @min1, %r2
+    // Transform %rN = LWZtoc @op1, %r2
     LowerPPCMachineInstrToMCInst(MI, TmpInst, *this, isDarwin);
 
-    // Change the opcode to LWZ, and the global address operand to be a
-    // reference to the GOT entry we will synthesize later.
+    // Change the opcode to LWZ.
     TmpInst.setOpcode(PPC::LWZ);
+
     const MachineOperand &MO = MI->getOperand(1);
+    assert(MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress() &&
+           "Unexpected operand type for LWZtoc pseudo.");
 
-    // Map symbol -> label of TOC entry
-    assert(MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress());
+    // Map the operand to its corresponding MCSymbol.
     MCSymbol *MOSymbol = nullptr;
     if (MO.isGlobal())
       MOSymbol = getSymbol(MO.getGlobal());
     else if (MO.isCPI())
       MOSymbol = GetCPISymbol(MO.getIndex());
     else if (MO.isJTI())
       MOSymbol = GetJTISymbol(MO.getIndex());
     else if (MO.isBlockAddress())
       MOSymbol = GetBlockAddressSymbol(MO.getBlockAddress());
 
-    if (PL == PICLevel::SmallPIC) {
+    const bool IsAIX = TM.getTargetTriple().isOSAIX();
+
+    if (PL == PICLevel::SmallPIC && !IsAIX) {
       const MCExpr *Exp =
         MCSymbolRefExpr::create(MOSymbol, MCSymbolRefExpr::VK_GOT,
                                 OutContext);
       TmpInst.getOperand(1) = MCOperand::createExpr(Exp);
-    } else {
-      MCSymbol *TOCEntry = lookUpOrCreateTOCEntry(MOSymbol);
+      EmitToStreamer(*OutStreamer, TmpInst);
+      return;
+    }
 
-      const MCExpr *Exp =
-        MCSymbolRefExpr::create(TOCEntry, MCSymbolRefExpr::VK_None,
-                                OutContext);
-      const MCExpr *PB =
-        MCSymbolRefExpr::create(OutContext.getOrCreateSymbol(Twine(".LTOC")),
-                                                             OutContext);
-      Exp = MCBinaryExpr::createSub(Exp, PB, OutContext);
+    // Otherwise use the TOC. 'TOCEntry' is a local symbol used to reference
+    // the storage allocated in the TOC which contains the address of
+    // 'MOSymbol'. Said TOC entry will be synthesized later.
+    MCSymbol *TOCEntry = lookUpOrCreateTOCEntry(MOSymbol);
+        MCSymbolRefExpr::create(TOCEntry, MCSymbolRefExpr::VK_None, OutContext);
+
+    // AIX uses the local symbol directly for the operand; that the symbol is
+    // accessed toc-relative is implicit.
+    if (IsAIX) {
+      assert(TM.getCodeModel() == CodeModel::Small &&
+             "This pseudo should only be selected for small code model.");
       TmpInst.getOperand(1) = MCOperand::createExpr(Exp);
+      EmitToStreamer(*OutStreamer, TmpInst);
+      return;
     }
+
+    // Create an explicit subtract expression between the local symbol and
+    // '.LTOC' to manifest the toc-relative offset.
+    const MCExpr *PB = MCSymbolRefExpr::create(
+        OutContext.getOrCreateSymbol(Twine(".LTOC")), OutContext);
+    Exp = MCBinaryExpr::createSub(Exp, PB, OutContext);
+    TmpInst.getOperand(1) = MCOperand::createExpr(Exp);
     EmitToStreamer(*OutStreamer, TmpInst);
     return;
   }
   case PPC::LDtocJTI:
   case PPC::LDtocCPT:
   case PPC::LDtocBA:
   case PPC::LDtoc: {
     // Transform %x3 = LDtoc @min1, %x2

llvm/test/CodeGen/PowerPC/lower-globaladdr32-aix-asm.ll

+; RUN: llc -mtriple powerpc-ibm-aix-xcoff \
+; RUN: -code-model=small < %s | FileCheck %s
+
+@b = common dso_local local_unnamed_addr global i32 0, align 4
+@a = common dso_local local_unnamed_addr global i32 0, align 4
+
+; Function Attrs: norecurse nounwind
+define dso_local void @test() local_unnamed_addr #0 {
+  %1 = load i32, i32* @b, align 4
+  store i32 %1, i32* @a, align 4
+  ret void
+}
+
+; CHECK-LABEL:   test
+; CHECK-DAG:       lwz [[REG1:[0-9]+]], LC0(2)
+; CHECK-DAG:       lwz [[REG2:[0-9]+]], LC1(2)
+; CHECK-DAG:       lwz [[REG3:[0-9]+]], 0([[REG1]])
+; CHECK:           stw [[REG3]], 0([[REG2]])
+; CHECK:           blr
+
+; TODO Update test when TOC-entry emission lands.
+; CHECK-NOT: .tc