[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:5367
+              S->getName(), XCOFF::XMC_PR, XCOFF::XTY_ER, XCOFF::C_EXT,
+              SectionKind::getMetadata());
+          S->setContainingCsect(Sec);
----------------
Xiangling_L wrote:
> @hubert.reinterpretcast  @cebowleratibm @sfertile @jasonliu Since we didn't get the chance to talk about this in the scrum, I am thinking I can put some of my thoughts here for discussion.
> 
> 1. I would treat GCC and LLVM 's behavior as a bug in this case. The compiler shouldn't synthesize to a `memcpy` which conflicts with user-defined `memcpy` and lead to some undefined behavior. The correct behavior seems should be when the compiler detect a user-defined/user-declared `memcpy`, it should give up the synthesis and keep original `load` and `store`.
> 
> Difficulties: We need a way to tell if there is a user-declared/user-defined `memcpy` in the translation unit when the compiler make the decision that if we want to replace a bunch of `load` and `store` with `memcpy` ExternalSymbolSDNode.
> 
> 2. On the other hand, if we want to follow GCC's behavior and try to pick up user-defined/user-declared `memcpy` to replace original bunch of `load` and `store`, we also need to way to tell if there is a user-declared/user-defined `memcpy` in the translation unit, based on which we set `Csect` accordingly.
> 
> In summary, it seems a way to tell if there is a user-declared/user-defined `memcpy` in the translation unit is a must, i.e. if there is a way to tell if there exists a `memcpy` GlobalObject. But so far I got no luck to figure out how to do that.  Anyone has any thoughts about that?
The LLVM and GCC behaviour is acceptable. Under C90 subclause 7.1.3, a user definition of `memcpy` leads to undefined behaviour. At the assembly and object generation level, we can handle cases where the user leaves `memcpy` as a function in a somewhat reasonable manner. If the user defines `memcpy` as an object, then we will preferably have a way to keep both `memcpy` symbols around even if the resulting assembly or object file is not exactly valid.

As for finding out whether the user has defined `memcpy`, I think we can look into `llvm::Module::getNamedValue`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70718