[PATCH] D119973: RegAllocGreedy: Fix last chance recolor assert in impossible case

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D119973#3397981 <https://reviews.llvm.org/D119973#3397981>, @arsenm wrote:

> In D119973#3394312 <https://reviews.llvm.org/D119973#3394312>, @arsenm wrote:
>
>> In D119973#3392579 <https://reviews.llvm.org/D119973#3392579>, @arsenm wrote:
>>
>>> In D119973#3392342 <https://reviews.llvm.org/D119973#3392342>, @qcolombet wrote:
>>>
>>>> The complicated cases are indeed not well tested since it is very hard to hit them. I was hoping Matt's fix would avoid falling in one of them, while not preventing actual success. Looks like it wasn't the case.
>>>>
>>>>> So e.g. if one of the evicted vregs get splitted, then the allocator may end up in a corrupted state after the physreg is restored to the splitted vreg. The vreg may then have an empty live range but still have a physreg allocated, and this may later lead to a number of different assertions and crashes.
>>>>
>>>> Is this what is happening here?
>>>> Would it be enough to drop the attempted recolored vreg when it has an empty range? (At this point, its live-range should be covered by the new split vregs)
>>>
>>> I'm still working on debugging this. I sort of see what's happening, but not really. At the point of failure, it's trying to unassign and reassign a register which was added to FixedRegisters after the initial interference check. It seems like the interference check needs to account for registers which will be added to FixedRegisters in the recursive calls. At the points those points are made, the ultimate parent register isn't in FixedRegisters for some reason.
>>
>> I can't articulate why, but changing the condition in mayRecolorAllInterferences to check if the stage is RS_Done or RS_Split also seems to work
>
> I think this just happens to hide it.
>
> The problem is when you have multiple registers being recolored, and one of them succeeds and the other fails recoloring. The one that succeeded did further recoloring which is not rolled back on the failure of the second. It leaves behind registers assigned to registers which were temporarily freed up in the parent call. This needs to track much more of the allocation state to accurately roll back the recoloring attempt

I now have another attempt which tracks a stack of successful recolorings that are rolled back on failure, rather than just the current depth's recolor candidates. However, the compile time is noticeably increased in my failing testcases (in a debug build one of them takes a full 15 seconds to complete) and it unexpectedly changes the output in a SystemZ test. I'm wondering if a better strategy would be to try avoiding recolor candidates that could conflict with the saved recoloring state for fixed registers


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

https://reviews.llvm.org/D119973