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

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 27 05:46:58 PDT 2020


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





More information about the llvm-commits mailing list