[PATCH] D92780: [InlineCost] Implement cost-benefit-based inliner

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:483
+  // The cost of cold basic blocks.
+  int ColdSize = 0;
+
----------------
can size be negative? Should it be int64_t?

Also, is it a size, or it's a cost - judging from its usage, probably the latter. Should it be ColdSizeCost?


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:674
+
+    auto CallerBB = CandidateCall.getParent();
+    BlockFrequencyInfo *CallerBFI =
----------------
it's auto*, right?


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:725
+      assert(ProfileCount.hasValue());
+      WeightedSavings += ProfileCount.getValue() * CurrentSavings;
+    }
----------------
for really large profile counts (e.g. worker thread that blocks, waiting for work) this may overflow. Do we care?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92780