[PATCH] D41675: Change memcpy/memove/memset to have dest and source alignment attributes.

[Daniel Neilson via Phabricator via llvm-commits]

dneilson marked an inline comment as done.
dneilson added inline comments.


================
Comment at: lib/IR/AutoUpgrade.cpp:526
+    if (Name.startswith("memcpy.") && F->arg_size() == 5) {
+      F->setName(Name + ".old");
+      // Get the types of dest, src, and len
----------------
arsenm wrote:
> dneilson wrote:
> > arsenm wrote:
> > > Is there a point to changing the name to something that will just be deleted?
> > Good question. This was in the original November 2015 change, and I just blindly copied it over.
> > 
> > I just tried removing the setName() calls, and am getting an assertion failure in the LLVM unit-test Linker/./LinkerTests/LinkModuleTest.RemangleIntrinsics:
> > 
> > ```
> > Assertion failed: (isa<X>(Val) && "cast<Ty>() argument of incompatible type!"), function cast, file /Users/dneilson/Development/LLVM-dev/llvm/include/llvm/Support/Casting.h, line 255.
> > ...
> > 8  LinkerTests              0x000000010ab03197 llvm::cast_retty<llvm::Function, llvm::Constant*>::ret_type llvm::cast<llvm::Function, llvm::Constant>(llvm::Constant*) + 103
> > 9  LinkerTests              0x000000010ac0b72e llvm::Intrinsic::getDeclaration(llvm::Module*, llvm::Intrinsic::ID, llvm::ArrayRef<llvm::Type*>) + 734
> > 10 LinkerTests              0x000000010aaa28b8 UpgradeIntrinsicFunction1(llvm::Function*, llvm::Function*&) + 21096
> > 11 LinkerTests              0x000000010aa9d578 llvm::UpgradeIntrinsicFunction(llvm::Function*, llvm::Function*&) + 40
> > 12 LinkerTests              0x000000010aac953b llvm::UpgradeCallsToIntrinsic(llvm::Function*) + 107
> > 13 LinkerTests              0x000000010a9bce8e llvm::LLParser::ValidateEndOfModule() + 10638
> > ```
> > 
> > I'm guessing that this is updating a declaration, and doesn't actually delete the declaration that it's replacing right away. So, renaming to append ".old" to the name effectively gets it out of the way and allows it to be deleted later on.
> It looks like this is how at least some places do this, but use the rename() function. This should probably do the same
Easy enough to change. Thanks for following up.


https://reviews.llvm.org/D41675