[PATCH] D84167: [Attributor] Use internalized version of non-exact functions

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 11 22:17:27 PDT 2020


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

LGTM, minor nits below.



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1430
+          (F.getLinkage() != llvm::GlobalValue::WeakAnyLinkage)) &&
+         "Trying to internalize function which cannot be internalized.");
+
----------------
Same as below.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2191
+          F->getLinkage() != llvm::GlobalValue::LinkOnceAnyLinkage &&
+          F->getLinkage() != llvm::GlobalValue::WeakAnyLinkage) {
+        Function *NewF = internalizeFunction(*F);
----------------
Please use `GlobalValue::isInterposableLinkage` here.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2202
+          }
+      }
+
----------------
Add a set of outer braces, this makes it easier to read.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1438
+      Function::Create(FnTy, llvm::GlobalValue::InternalLinkage,
+                       F.getAddressSpace(), F.getName() + "_copied");
+  ValueToValueMapTy VMap;
----------------
jdoerfert wrote:
> Maybe: ".internalized" ?
We actually want `PrivateLinkage` I think. No need for `llvm::` here and elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84167



More information about the llvm-commits mailing list