[PATCH] D84614: [Attributor][WIP] Cost interface for function internalization

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:145
+
+} // namespace InternalizeConstants
+
----------------
Make the names more descriptive and add plenty doxygen documentation for all of them. Maybe put them all in a struct so the user could modify the values by changing the struct members.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2201
     for (Function *F : Functions)
-      if (!F->hasExactDefinition() && F->getNumUses() &&
-          F->getLinkage() != llvm::GlobalValue::LinkOnceAnyLinkage &&
-          F->getLinkage() != llvm::GlobalValue::WeakAnyLinkage) {
+      if (A.getInternalizeCost(*F)) {
         Function *NewF = internalizeFunction(*F);
----------------
I somehow think the method called here should be "shouldInternalize" or something. The problem here is that it "looks" like we internalize if there is an associated cost, e.g., the cost is not zero.


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

https://reviews.llvm.org/D84614