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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5366
+          MCSectionXCOFF *Sec = Context.getXCOFFSection(
+              S->getName(), XCOFF::XMC_PR, XCOFF::XTY_ER, XCOFF::C_EXT,
+              SectionKind::getMetadata());
----------------
Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > What happens if the user is defining `memset` in this file?
> I did some test, and I think it's worth bringing up the findings in the srum, but I will document what I found here.
> 
> 
> ```
> [test_user_define_memcpy.c]
> int memcpy ( void * destination, int num ) { return 3; }
> 
> typedef struct cpy_array {
>   int array[20];
> } cpy;
> 
> int main() {
>   cpy A = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,16};
>   cpy B = A;
>   return B.array[0];
> }
> 
> ```
> 1. XLC on AIX with no-opt:  
> XLC doesn't synthesize load & store to `memcpy`, the result is correct `=1`.
> 
> 2. GCC on AIX with no-opt:
> GCC in this case didn't pick up library version `memcpy`, and the result was some random garbage value.
> 
> 3. LLVM with `powerpc64le-unknown-linux-gnu` target with no-opt:
> the same as above GCC case.
> 
> So I think the following questions are:
> 1) Is GCC and LLVM's behavior in this case a bug?
> 2) Do we want to follow GCC's behavior?
> 3) If the answer of `2)` is yes, how do we want to achieve that, especially when the definition of `memcpy` is put after `callInst` of `ExternalSymbolSDNode`, like the following:
> 
> 
> ```
> typedef struct cpy_array {
>   int array[20];
> } cpy;
> 
> int main() {
>   cpy A = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,16};
>   cpy B = A;
>   return B.array[0];
> }
> 
> int memcpy ( void * destination, int num ) { return 3; }
> 
> ```
Regardless of whether the user-defined or the system-defined version gets called (I believe either is okay), we should make sure that the lookup and possible creation of the symbol here interacts reasonably with the definition or a user-declaration that changes the symbol properties (such as marking the function weak).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70718