Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regalloc basic asserts with "Interference after spill." #32404

Closed
JonPsson opened this issue May 16, 2017 · 7 comments
Closed

Regalloc basic asserts with "Interference after spill." #32404

JonPsson opened this issue May 16, 2017 · 7 comments
Labels
bugzilla Issues migrated from bugzilla llvm:regalloc

Comments

@JonPsson
Copy link
Contributor

Bugzilla Link 33057
Resolution FIXED
Resolved on Jun 02, 2017 15:44
Version trunk
OS Linux
Attachments reduced testcase
CC @JonPsson,@qcolombet

Extended Description

llc -mtriple=s390x-linux-gnu -mcpu=z13 -disable-cgp -disable-machine-dce -regalloc=basic -o /dev/null ./tc_regallocbasic.ll

llc: llvm/lib/CodeGen/RegAllocBasic.cpp:253: virtual unsigned int {anonymous}::RABasic::selectOrSplit(llvm::LiveInterval&, llvm::SmallVectorImpl&): Assertion `!Matrix->checkInterference(VirtReg, *PhysRegI) && "Interference after spill."' failed.

@qcolombet
Copy link
Collaborator

Looking at the regalloc dump I see this:
selectOrSplit GR64Bit:%vreg123 [1944r,1952r:0) 0@1944r w=INF
spilling R1D interferences with %vreg123 [1944r,1952r:0) 0@1944r
unassigning %vreg96 from %R1L: R1L
Inline spilling GR32Bit:%vreg96 EMPTY

Thus, vreg123 interferes with vreg96. However, vreg96 is an empty live-range so it shouldn't interfere to being with. Going pass this shock, the fact that vreg96 is empty implies that the liveregmatrix does not need to update anything when unassigning it. Therefore the interference stays and boom!

The bottom line is why this vreg96 live-range didn't get removed from the liveregmatrix.

@qcolombet
Copy link
Collaborator

I found it suspicious that RABasic is not a delegate of LiveRangeEdit.
Let me see if that's problem.

@qcolombet
Copy link
Collaborator

Possible fix
Yes, that's a problem.
Attached a quick patch to fix it.

The proper way to fix that is IMHO to do that in the base class so that we don't duplicate the code between RABasic and RAGreedy.

@qcolombet
Copy link
Collaborator

One problem with the proposed fix is that it changes the behavior of the basic allocator. Now, each time a register live-range is shrunk, the live-range is re-queued for assignment.

Previously we would shrink the live-range but not give back the register for the holes we created.
This means two things:

  • Compile time overhead: More live-range will be processed
  • Regalloc choices diffs

I am guessing that's not too concerning.
With the current LiveRegMatrix API, to preserve the behavior, we would need to do something like:

  1. unassign the live-range to X
  2. shrink the live-range
  3. reassign the shrunk live-ranges to X

Right now, we cannot do #​3 because of:
A. the lack of call back (fixable)
B. the coupling/state will would introduce between 1, 2 and 3

Because of #B, I don't think we should try to fix that.

@JonPsson
Copy link
Contributor Author

JonPsson commented Jun 1, 2017

It seems all good to me, but I think someone with more insight should be the reviewer.

@qcolombet
Copy link
Collaborator

It seems all good to me, but I think someone with more insight should be the
reviewer.

Don't worry, I'll commit shortly, I need to clean-up the test case. (Producing the mir and adding check lines.)

@qcolombet
Copy link
Collaborator

Fixed in r304603.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla llvm:regalloc
Projects
None yet
Development

No branches or pull requests

2 participants