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

[Aditya Kumar via Phabricator via llvm-commits]

hiraditya added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/HotColdSplitting.cpp:88
 
+static cl::opt<int> MaxParametersForSplit(
+    "hotcoldsplit-max-params", cl::init(8), cl::Hidden,
----------------
vsk wrote:
> 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.
yes


================
Comment at: llvm/lib/Transforms/IPO/HotColdSplitting.cpp:309
+  unsigned NumSplitExitPhis = 0;
+  for (BasicBlock *ExitBB : SuccsOutsideRegion) {
+    for (PHINode &PN : ExitBB->phis()) {
----------------
vsk wrote:
> 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).
>  I've never observed a performance issue here (in practice |SuccsOutsideRegion| is very small).

In that case the current approach is fine.


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

https://reviews.llvm.org/D59715