[PATCH] D125102: [RegAllocGreedy] New hook regClassPriorityTrumpsGlobalness

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocGreedy.cpp:291
   unsigned Prio;
 
   auto Stage = ExtraInfo->getOrInitStage(Reg);
----------------
Just for info:

Downstream we are doing
```
 if (<our downstream target>) {
    // Let AllocationPriority affect all ranges.
    const TargetRegisterClass &RC = *MRI->getRegClass(Reg);
    Size = Size * (RC.AllocationPriority + 1);
  }
```
here (I think Quentin have suggested something like that in the past).

Anyway, I tried replacing that old hack by this patch, but got some mixed results. Same thing if using both patches together.

Last time I tried to do something in this area I had a hard time finding some heuristic that gave generally better result without some occasional larger regression. Kind of annoying, since your patch also seem to indicate that there is a potential gain here also for our target in several benchmarks (but regressions by almost 20% in a couple of our benchmarks can't be ignored completely). That might of course be due to other shortcomings in our backend (such as the AllocationPriority setup etc). I guess I need to investigate that a bit closer before we consider to use this new option for our target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125102