[PATCH] D125102: [RegAllocGreedy] New hook regClassPriorityTrumpsGlobalness

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/CodeGen/RegAllocGreedy.cpp:291
   unsigned Prio;
 
   auto Stage = ExtraInfo->getOrInitStage(Reg);
----------------
bjope wrote:
> 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.
Thanks for trying it. Does your downstream target also have a concept of occupancy, so that using fewer physical registers means that things run significantly faster on the hardware? I'm aware that the register allocator was developed for CPUs and CPUs generally do not have that property.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125102