[PATCH] D125102: [RegAllocGreedy] New hook regClassPriorityTrumpsGlobalness

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetRegisterInfo.h:1081-1087
+  /// When prioritizing live ranges in register allocation, if this hook returns
+  /// true then the AllocationPriority of the register class will be treated as
+  /// more important than whether the range is local to a basic block or global.
+  virtual bool
+  regClassPriorityTrumpsGlobalness(const MachineFunction &MF) const {
+    return false;
+  }
----------------
foad wrote:
> arsenm wrote:
> > arsenm wrote:
> > > What's the argument for making this configurable? No lit tests fail if I default this to true?
> > Basically I think the regclass priority was just a broken feature before, and this is a flag to enable/disable a bug fix
> > No lit tests fail if I default this to true?
> 
> Really? I get:
> ```
> Failed Tests (43):
>   LLVM :: CodeGen/AMDGPU/GlobalISel/insertelement.ll
>   LLVM :: CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.div.fmas.ll
>   LLVM :: CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.wqm.demote.ll
>   LLVM :: CodeGen/AMDGPU/GlobalISel/localizer.ll
>   LLVM :: CodeGen/AMDGPU/GlobalISel/sdiv.i64.ll
>   LLVM :: CodeGen/AMDGPU/GlobalISel/srem.i64.ll
>   LLVM :: CodeGen/AMDGPU/agpr-copy-no-free-registers.ll
>   LLVM :: CodeGen/AMDGPU/amdgpu-codegenprepare-idiv.ll
>   LLVM :: CodeGen/AMDGPU/atomic_optimizations_buffer.ll
>   LLVM :: CodeGen/AMDGPU/atomic_optimizations_global_pointer.ll
>   LLVM :: CodeGen/AMDGPU/atomic_optimizations_local_pointer.ll
>   LLVM :: CodeGen/AMDGPU/atomic_optimizations_pixelshader.ll
>   LLVM :: CodeGen/AMDGPU/atomic_optimizations_raw_buffer.ll
>   LLVM :: CodeGen/AMDGPU/atomic_optimizations_struct_buffer.ll
>   LLVM :: CodeGen/AMDGPU/collapse-endcf.ll
>   LLVM :: CodeGen/AMDGPU/ctpop16.ll
>   LLVM :: CodeGen/AMDGPU/dag-divergence-atomic.ll
>   LLVM :: CodeGen/AMDGPU/divergent-branch-uniform-condition.ll
>   LLVM :: CodeGen/AMDGPU/extend-phi-subrange-not-in-parent.mir
>   LLVM :: CodeGen/AMDGPU/extract-subvector-16bit.ll
>   LLVM :: CodeGen/AMDGPU/fix-frame-ptr-reg-copy-livein.ll
>   LLVM :: CodeGen/AMDGPU/global-atomics-fp.ll
>   LLVM :: CodeGen/AMDGPU/i1-copy-from-loop.ll
>   LLVM :: CodeGen/AMDGPU/idiv-licm.ll
>   LLVM :: CodeGen/AMDGPU/insert_vector_dynelt.ll
>   LLVM :: CodeGen/AMDGPU/llvm.amdgcn.wqm.demote.ll
>   LLVM :: CodeGen/AMDGPU/llvm.round.f64.ll
>   LLVM :: CodeGen/AMDGPU/load-constant-i16.ll
>   LLVM :: CodeGen/AMDGPU/loop_break.ll
>   LLVM :: CodeGen/AMDGPU/mul24-pass-ordering.ll
>   LLVM :: CodeGen/AMDGPU/no-dup-inst-prefetch.ll
>   LLVM :: CodeGen/AMDGPU/sdiv64.ll
>   LLVM :: CodeGen/AMDGPU/sgpr-control-flow.ll
>   LLVM :: CodeGen/AMDGPU/si-annotate-cf-kill.ll
>   LLVM :: CodeGen/AMDGPU/skip-if-dead.ll
>   LLVM :: CodeGen/AMDGPU/spill-vgpr.ll
>   LLVM :: CodeGen/AMDGPU/srem64.ll
>   LLVM :: CodeGen/AMDGPU/udiv64.ll
>   LLVM :: CodeGen/AMDGPU/urem64.ll
>   LLVM :: CodeGen/AMDGPU/wqm.ll
>   LLVM :: CodeGen/PowerPC/more-dq-form-prepare.ll
>   LLVM :: CodeGen/PowerPC/ppc64-acc-regalloc.ll
>   LLVM :: CodeGen/PowerPC/subreg-killed.mir
> ```
> 
> > Basically I think the regclass priority was just a broken feature before, and this is a flag to enable/disable a bug fix
> 
> I don't know why you call it a bug. It was just a different heuristic. I'm pretty sure I could find cases that get better allocation with either setting of the flag if I went looking for them.
> I'm pretty sure I could find cases that get better allocation with either setting of the flag if I went looking for them.

798fa7e9d6973c7ecb736eea41755ef86220cda1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125102