[PATCH] D30422: Keep attributes, calling convention, etc, when remangling intrinsic

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: lib/IR/AutoUpgrade.cpp:1915
   std::string Name = CI->getName();
-  if (!Name.empty())
-    CI->setName(Name + ".old");
-
+  auto *Repl = CI;
   switch (NewFn->getIntrinsicID()) {
----------------
How about `NewCall` instead of `Repl`?


================
Comment at: lib/IR/AutoUpgrade.cpp:2008
     CallInst *NewCall = Builder.CreateCall(NewFn, {BC0, BC1}, Name);
-    CI->replaceAllUsesWith(NewCall);
-    CI->eraseFromParent();
-    return;
+    Repl = NewCall;
+    break;
----------------
These should just be `Repl = Builder.CreateCall(NewFn, {BC0, BC1}, Name);`


================
Comment at: lib/IR/AutoUpgrade.cpp:2048
+    if (!Name.empty())
+      CI->setName(Name + ".old");
+    CI->replaceAllUsesWith(Repl);
----------------
I'm not sure if this does what used to happen before.  To get the behavior intended by https://reviews.llvm.org/rL146357 I think `CI->setName(Name + ".old");` needs to happen //before// you create a new call instruction with `Name` as name.  Either that, or some shuffling as:

```
if (Name not empty) {
  CI->setName(Name + ".old");
  Repl->setName(Name); // This one should "stick"
  ...
}
```



https://reviews.llvm.org/D30422