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

[Jason Liu via Phabricator via llvm-commits]

jasonliu added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5185
+    const Module *Mod = DAG.getMachineFunction().getFunction().getParent();
+    if (const GlobalValue *GV = Mod->getNamedValue(SymName)) {
+      const GlobalObject *GO = dyn_cast<GlobalObject>(GV);
----------------
This is returning a GlobalValue, which means it could also be a global variable. If so, I don't think we want to enter this branch. 


================
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 \
----------------
do we care to put -mcpu=pwr4 in this test case?


================
Comment at: llvm/test/CodeGen/PowerPC/aix-user-defined-memcpy.ll:7
+
+define dso_local signext i32 @memcpy(i8* %destination, i32 signext %num) {
+entry:
----------------
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. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70718