[PATCH] D66047: Do not call replaceAllUsesWith to upgrade calls to ARC runtime functions to intrinsic calls

[Akira Hatanaka via Phabricator via llvm-commits]

ahatanak marked 2 inline comments as done.
ahatanak added inline comments.


================
Comment at: llvm/lib/IR/AutoUpgrade.cpp:3857
 
 void llvm::UpgradeARCRuntimeCalls(Module &M) {
+  // This lambda converts normal function calls to ARC runtime functions to
----------------
steven_wu wrote:
> I probably missed this from previous commit, shouldn't this function return `bool`?
I changed the return type to void in a follow-up commit since nothing seemed to be using the return value. Do these functions have to return anything?


================
Comment at: llvm/lib/IR/AutoUpgrade.cpp:3871
+      if (!CI || CI->getCalledFunction() != Fn)
+        continue;
+
----------------
rjmccall wrote:
> Intrinsic functions can't be used in any way except calling them, right?
Intrinsic functions can only be called, but it's possible to use ObjC ARC functions without calling it. For example, if the code below was compiled by clang and the auto-upgrader was reading the bitcode, the check would prevent the auto-upgrader from replacing the reference to `@objc_autorelease`:

```
id objc_autorelease(id);

void foo2(void *);

void foo1(void) {
  //   call void @foo2(i8* bitcast (i8* (i8*)* @objc_autorelease to i8*))
  foo2(&objc_autorelease);
}
```

I'm not sure why anyone would do something like this, but currently clang doesn't reject it and I don't think we can rule out the possibility that someone is generating an IR like this.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66047