[PATCH] D68341: [AIX] TOC pseudo expansion for 64bit large + 64bit small + 32bit large modes

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast marked an inline comment as done.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM with minor changes.



================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCInstPrinter.cpp:82
+           "The third operand of an addis instruction should be a symbol "
+	   "reference expression if it is an expression at all.");
+
----------------
Indentation:
```
    assert(isa<MCSymbolRefExpr>(MI->getOperand(2).getExpr()) &&
           "The third operand of an addis instruction should be a symbol "
           "reference expression if it is an expression at all.");
```


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:734
   case PPC::LDtoc: {
+    assert (!IsDarwin && "TOC is an ELF/XCOFF construct");
+
----------------
The space after `assert` seems odd. `clang-format` removes this space.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:766
+    // Transform %rd = ADDIStocHA %rA, @sym(%r2)
+    LowerPPCMachineInstrToMCInst(MI, TmpInst, *this, IsDarwin);
+
----------------
We know `IsDarwin` is false here. For code readability purposes, I am okay with this line as-is because the Darwin code is supposedly going away.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:821
   case PPC::ADDIStocHA8: {
+    assert (!IsDarwin && "TOC is an ELF/XCOFF construct");
+
----------------
Same comment about the space.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:860
   case PPC::LDtocL: {
+    assert (!IsDarwin && "TOC is an ELF/XCOFF construct");
+
----------------
Same comment about the space.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:332
   case PPC::ADDIStocHA8:
+  case PPC::ADDIStocHA:
   case PPC::ADDItocL:
----------------
This patch adds `ADDIStocHA` to two switches with a different ordering relative to `ADDIStoHA8` in each switch.


================
Comment at: llvm/test/CodeGen/PowerPC/lower-globaladdr32-aix-asm.ll:15
+
+; SMALL-LABEL: test_load
+; SMALL: lwz [[REG1:[0-9]+]], LC0(2)
----------------
This is a function entry point label, so
```
SMALL-LABEL: .test_load:{{$}}
```
is appropriate.



================
Comment at: llvm/test/CodeGen/PowerPC/lower-globaladdr32-aix-asm.ll:20
+
+; LARGE-LABEL: test_load
+; LARGE: addis [[REG1:[0-9]+]], LC0 at u(2)
----------------
Same comment re: function entry label.


================
Comment at: llvm/test/CodeGen/PowerPC/lower-globaladdr32-aix-asm.ll:33
 
-; 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
+; SMALL-LABEL: test_store
+; SMALL: lwz [[REG1:[0-9]+]], LC1(2)
----------------
Same comment re: function entry label.


================
Comment at: llvm/test/CodeGen/PowerPC/lower-globaladdr32-aix-asm.ll:38
+
+; LARGE-LABEL: test_store
+; LARGE: addis [[REG1:[0-9]+]], LC1 at u(2)
----------------
Same comment re: function entry label.


================
Comment at: llvm/test/CodeGen/PowerPC/lower-globaladdr64-aix-asm.ll:15
+
+; SMALL-LABEL: test_load
+; SMALL: ld [[REG1:[0-9]+]], LC0(2)
----------------
Same comment re: function entry label.


================
Comment at: llvm/test/CodeGen/PowerPC/lower-globaladdr64-aix-asm.ll:17
+; SMALL: ld [[REG1:[0-9]+]], LC0(2)
+; SMALL: lwz [[REG2:[0-9]+]], 0([[REG1]])
+; SMALL: blr
----------------
Just a note:
Currently, the front-end seems to not generate `signext` or `zeroext` into the IR for 64-bit AIX. It seems the default behaviour matches `zeroext` (meaning the return type is unsigned).


================
Comment at: llvm/test/CodeGen/PowerPC/lower-globaladdr64-aix-asm.ll:20
+
+; LARGE-LABEL: test_load
+; LARGE: addis [[REG1:[0-9]+]], LC0 at u(2)
----------------
Same comment re: function entry label.


================
Comment at: llvm/test/CodeGen/PowerPC/lower-globaladdr64-aix-asm.ll:33
+
+; SMALL-LABEL: test_store
+; SMALL: ld [[REG1:[0-9]+]], LC1(2)
----------------
Same comment re: function entry label.


================
Comment at: llvm/test/CodeGen/PowerPC/lower-globaladdr64-aix-asm.ll:38
+
+; LARGE-LABEL: test_store
+; LARGE: addis [[REG1:[0-9]+]], LC1 at u(2)
----------------
Same comment re: function entry label.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68341