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

[Xiangling Liao via Phabricator via llvm-commits]

Xiangling_L marked 13 inline comments as done.
Xiangling_L added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-external-sym-sdnode-lowering.ll:1
+; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff \
+; RUN: -stop-after=machine-cp < %s | FileCheck \
----------------
jasonliu wrote:
> do we care to put -mcpu=pwr4 in this test case?
I didn't add `-mcpu=pwr4` because not all ExternalSDNode we want to test can be generated under a specific cpu level. And I think this testcase is more focused on, if xxx` ExternalSymbolSDNode synthesized by compiler, we are able to map it to `.xxx`. So it doesn't really matter to add cpu level as well.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-user-defined-memcpy.ll:7
+
+define dso_local signext i32 @memcpy(i8* %destination, i32 signext %num) {
+entry:
----------------
jasonliu wrote:
> Maybe it's worth to explain what this test is for? For example, to make sure under this circumstances we would still set up the right memcpy symbol to call the user defined memcpy instead of external reference memcpy. 
> The information are all in the review comments which is good, but it would be great if people don't need to dig up the original review to find them. 
Good point, I will add some comments in the testcase.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-user-defined-memcpy.ll:20
+
+; CHECK: bl .memcpy
+; CHECK-NOT: error
----------------
hubert.reinterpretcast wrote:
> This test should preferably check the symbol table for a `.o` file and the relocation associated with the call.
Good point, I will add it in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70718