This is an archive of the discontinued LLVM Phabricator instance.

[NFC][regalloc] Factor eviction decision-making into an analysis
Needs ReviewPublic

Authored by mtrofin on Nov 9 2021, 3:58 PM.

Details

Reviewers
qcolombet
davidxl
Summary

The goal is to support alternative policies for eviction, e.g. via
ML-trained policy. Such alternatives would be made available as
alternative implementations of RegAllocEvictionAdvisor, and the
RegAllocEvictionAdvisorAnalysis would make the switch between them.

The bulk of the change moves code from RegAllocGreedy.cpp to the
advisor. The change also renames canEvictInterference to
shouldEvictInterference to suggest this is a question of
profitability, not necessarily correctness.

RFC: https://lists.llvm.org/pipermail/llvm-dev/2021-November/153639.html

Diff Detail

Event Timeline

mtrofin created this revision.Nov 9 2021, 3:58 PM
mtrofin requested review of this revision.Nov 9 2021, 3:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 9 2021, 3:58 PM
mtrofin edited the summary of this revision. (Show Details)Nov 9 2021, 4:02 PM
lkail added a subscriber: lkail.Nov 9 2021, 5:01 PM

gentle reminder.

if the reviewers preferred, I could split this in a few CLs (albeit it's really just a code move from one file to the other) - wdyt?

gentle reminder.

if the reviewers preferred, I could split this in a few CLs (albeit it's really just a code move from one file to the other) - wdyt?

Gentle reminder - thanks!

To minimize text diff, may I suggest the following steps:

  1. split out the changes that mechanically move the common declarations (such as enums) into the header
  1. do not introduce the new .cpp file in the patch, but instead, leave the Advisor code in the original file. For instance EvictAdvisor::CanReassign 's definition stays in RegAllocGreedy.cpp.
  1. As a follow up, mechanically move the functions to a new cpp file.

This makes reviewing 2) more easily.

To minimize text diff, may I suggest the following steps:

  1. split out the changes that mechanically move the common declarations (such as enums) into the header
  1. do not introduce the new .cpp file in the patch, but instead, leave the Advisor code in the original file. For instance EvictAdvisor::CanReassign 's definition stays in RegAllocGreedy.cpp.
  1. As a follow up, mechanically move the functions to a new cpp file.

This makes reviewing 2) more easily.

Makes sense - I'll start in order, then, referencing the RFC and then progressively rebasing this.

wenlei added a subscriber: wenlei.Nov 19 2021, 11:43 AM

This patch is obsolete now, or still valid after rebasing?

This patch is obsolete now, or still valid after rebasing?

Obsolete - I'm just using it as an umbrella to collect all the patches that ended up happening, like we did with the inliner work.