Created attachment 18450 [details] reduced testcase 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<unsigned int>&): Assertion `!Matrix->checkInterference(VirtReg, *PhysRegI) && "Interference after spill."' failed.
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.
I found it suspicious that RABasic is not a delegate of LiveRangeEdit. Let me see if that's problem.
Created attachment 18552 [details] 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.
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.
It seems all good to me, but I think someone with more insight should be the reviewer.
(In reply to Jonas Paulsson from comment #5) > 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.)
Fixed in r304603.