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

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/IR/AutoUpgrade.cpp:1924
         "Unknown function for CallInst upgrade and isn't just a name change");
-    SmallVector<Value *, 4> Args(CI->arg_operands().begin(),
-                                 CI->arg_operands().end());
-    CI->replaceAllUsesWith(Builder.CreateCall(NewFn, Args));
-    CI->eraseFromParent();
+    CI->setCalledFunction(NewFn);
     return;
----------------
sanjoy wrote:
> I think we also need to undo the name change.  Setting it and then unsetting it seems a bit hacky, perhaps we could have the switch create an unnamed `NewCI`, and outside it do:
> 
> ```
> if (NewCI != CI) {
>   // Adjust names, and kill CI
> }
> ```
> 
> (and remove the pre-rename)?
Just out of curiosity, why?
I mean, it renames it to old, but who cares?

It is just the name of the call instruction, not the actual called function?

IE it's just changing

%1 = call foo
into
%1.old = call foo


We could call it ".upgraded" instead?
:)


Actually, looking closer, all the other paths erase CI completely or leave it alone, so i'm not sure why both changing the name at all.
It looks like we are just wasting time?

IE all the paths before it depend on !NewFn and then destroy it, even if there is no replacement. 
So if we get to the renaming, we've done nothing to it.

All the paths after either do nothing, erase CI completely, or are the one we are talking about.

Thus it seems we could just leave the name alone and declare victory?

Or am i missing something?



https://reviews.llvm.org/D30422