[PATCH] D70718: [AIX] Replace ExternalSymbolSDNode with a MCSymbolSDNode for memcpy/memset/memmove

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added a subscriber: DiggerLin.
hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5201
+    if (const GlobalValue *GV = Mod->getNamedValue(SymName)) 
+      if (const Function *F = dyn_cast<Function>(GV)) {
+        const XCOFF::StorageClass SC =
----------------
The two `if`s can be folded using `dyn_cast_or_null`.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-user-defined-memcpy.ll:2
+; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 \
+; RUN: -mattr=-altivec -filetype=obj -o %t.o < %s | llvm-readobj --syms %t.o | \
+; RUN: FileCheck --check-prefix=32-SYM %s
----------------
There should be no need to pipe into `llvm-readobj`. Place the split this into separate RUN lines between the `.o` generation and the inspection of the `.o`.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-user-defined-memcpy.ll:5
+
+; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 \
+; RUN: -mattr=-altivec -filetype=obj -o %t.o < %s | not llvm-readobj --r %t.o \
----------------
You only need to produce the `.o` file once (in this case, on the first RUN line of the file).


================
Comment at: llvm/test/CodeGen/PowerPC/aix-user-defined-memcpy.ll:31
+
+; TODO: This test should preferably check the symbol table for a.o file and
+;       the relocation associated with the call.
----------------
Replace "a.o file" with "a `.o` file".


================
Comment at: llvm/test/CodeGen/PowerPC/aix-user-defined-memcpy.ll:34
+
+; 32-SYM:       Symbol {
+; 32-SYM:         Index: 2
----------------
Use the `[[:space:]]` formulation (@DiggerLin can help you) so we can allow the symbol table index to change.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-user-defined-memcpy.ll:53
+; 32-SYM-NEXT:    }
+; 32-SYM-NEXT:  }
+
----------------
Add a `32-SYM-NOT` against further symbols named `.memcpy`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70718