[PATCH] D105118: [remangleIntrinsicFunction] Detect and resolve name clash

[Artur Pilipenko via Phabricator via llvm-commits]

apilipenko accepted this revision.
apilipenko added a comment.
This revision is now accepted and ready to land.

Looks good.

May I ask you to document the new side effect (possible renaming of another function in the Module) of remangleIntrinsicFunction in the corresponding header?



================
Comment at: llvm/lib/IR/Function.cpp:1666-1682
+  Function *NewDecl = nullptr;
+  if (auto *ExistingGV = F->getParent()->getNamedValue(WantedName)) {
+    if (auto *ExistingF = dyn_cast<Function>(ExistingGV))
+      if (ExistingF->getFunctionType() == F->getFunctionType())
+        NewDecl = ExistingF;
+
+    if (!NewDecl) {
----------------
Stylistic suggestions. This can be restructured to use a lambda and early exits. Something like this:
```
  Function *NewDecl = [&] () {
    if (auto *ExistingGV = F->getParent()->getNamedValue(WantedName)) {
      if (auto *ExistingF = dyn_cast<Function>(ExistingGV))
        if (ExistingF->getFunctionType() == F->getFunctionType())
          return ExistingF;

      // The name already exists, but is not a function or has the wrong
      // prototype. Make place for the new one by renaming the old version.
      // Either this old version will be removed later on or the module is
      // invalid and we'll get an error.
      ExistingGV->setName(WantedName + ".renamed");
    }
  
    return Intrinsic::getDeclaration(F->getParent(), ID, ArgTys);
  }();
```


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

https://reviews.llvm.org/D105118