Page MenuHomePhabricator

[RegisterCoalescer] passs Undefs to extendToIndices()
Needs ReviewPublic

Authored by ruiling on Tue, Sep 15, 11:42 PM.

Details

Summary

When extending the subranges, the reaching-def may be an undefs. When
extending such kind of subrange, it will try to search for the reaching
def first. If the reaching def is an undef and we did not provide 'Undefs',
The findReachingDefs() will fail with message:
"Use of $noreg does not have a corresponding definition on every path:
LLVM ERROR: Use not jointly dominated by defs."
So we computeSubRangeUndefs() and pass the result to extendToIndices().

Diff Detail

Event Timeline

ruiling created this revision.Tue, Sep 15, 11:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Sep 15, 11:42 PM
ruiling requested review of this revision.Tue, Sep 15, 11:42 PM
arsenm added inline comments.Wed, Sep 16, 7:07 AM
llvm/test/CodeGen/AMDGPU/coalescer-removepartial-extend-undef-subrange.mir
21

$noreg doesn't have liveness?

30

Can you reduce this any further? -run-pass=none will also compact the register numbers

Typo passs in commit message

ruiling added inline comments.Wed, Sep 16, 6:34 PM
llvm/test/CodeGen/AMDGPU/coalescer-removepartial-extend-undef-subrange.mir
21

I am not sure what you are really asking here. I know the assert message is a little bit misleading. The reason it output $noreg is we are passing 0 to argument 'PhyReg' of LiveRangeCalc::extend() in LiveIntervals::extendToIndices(). When assert happens, we are trying to extend a subrange of register %90. The offending subrange has a def point in one of bb.4 predecessor chain, so it has liveness in this path. but in another predecessor chain 4->27->1->0, it only has one undef point in bb.0.

30

I just tried to remove each one of the most blocks containing instructions in MIR, I failed to trigger the crash after removing them. If you have any good idea to simplify further, I can try it. Some more notes: The test case is already a much simplified version than original failure IR with 100+ basic blocks. I have tried removing each basic block in IR before and it would cause problem disappear. The problem is hard to trigger, it also depends on the order of coalescing work. For example, if I reorder the blocks, the problem may disappear because the coalescing of registers will be different then.

ruiling updated this revision to Diff 292404.Wed, Sep 16, 8:50 PM

address some review comments from Matt

arsenm added inline comments.Thu, Sep 17, 7:29 AM
llvm/test/CodeGen/AMDGPU/coalescer-removepartial-extend-undef-subrange.mir
30

I've reduced coalescer cases by dumping the MIR at intermediate points during the coalescer. Presumably not every step is relevant to get to the problem

ruiling updated this revision to Diff 292685.Thu, Sep 17, 5:55 PM

further reduce .mir with intermediate result from coalescer.

ruiling updated this revision to Diff 292706.Thu, Sep 17, 10:31 PM

further reduce .mir to trigger the issue. @arsenm Could you take a look?