[PATCH] D59715: [HotColdSplit] Reflect full cost of parameters in split penalty

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

I haven't forgotten about this! Hope to have an update by Monday (8/10).



================
Comment at: llvm/lib/Transforms/IPO/HotColdSplitting.cpp:88
 
+static cl::opt<int> MaxParametersForSplit(
+    "hotcoldsplit-max-params", cl::init(8), cl::Hidden,
----------------
hiraditya wrote:
> Should we initialize this to be same as # of argument registers. Any arguments more than supported by argument registers are generally stored on stack and may add to code-size overhead both in caller and callee.
Ah, do you mean the number of arguments that can be passed by register? If so, yes, that seems like a straightforward improvement.


================
Comment at: llvm/lib/Transforms/IPO/HotColdSplitting.cpp:309
+  unsigned NumSplitExitPhis = 0;
+  for (BasicBlock *ExitBB : SuccsOutsideRegion) {
+    for (PHINode &PN : ExitBB->phis()) {
----------------
hiraditya wrote:
> Do we have a limit on max # successors outside of the region? In pathological cases this piece of code may have super linear time complexity. If we dont encounter this often then this seems fine though.
No, we don't have a limit like that, but I'd be open to adding one if a pathological case is found. I've never observed a performance issue here (in practice |SuccsOutsideRegion| is very small).


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

https://reviews.llvm.org/D59715