[PATCH] D132835: [NFC][Regalloc] Introduce the RegAllocPriorityAdvisorAnalysis

Mircea Trofin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 29 13:50:03 PDT 2022


mtrofin added a comment.

overall lgtm. in addition to the comments, also add a test similar to https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/MLRegalloc/default-eviction-advisor.ll - the idea is to check that, indeed, if the user asks for the unsupported advisor, the error message is given.



================
Comment at: llvm/lib/CodeGen/RegAllocGreedy.cpp:2572
+  PriorityAdvisor = getAnalysis<RegAllocPriorityAdvisorAnalysis>().getAdvisor(
+      *MF, *this, Indexes, RegClassPriorityTrumpsGlobalness,
+      ReverseLocalAssignment);
----------------
I think Indexes is an analysis you could just get as a dependency of the regallocpriorityadvisoranalysis. Then that'd make the constructor simpler - ideally it would be reduced to just `*MF` and `*this` - it'll make it easier to evolve: we wouldn't need to touch N derived classes every time a N+1 one would need some new parameter - they could just get it from either the greedy allocator, or the analysis manager.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132835



More information about the llvm-commits mailing list