This is an archive of the discontinued LLVM Phabricator instance.

Fix register coalescer failure to prune value
ClosedPublic

Authored by rampitec on May 20 2019, 2:41 PM.

Details

Summary

Register coalescer fails for the test in the patch with the assertion in
JoinVals::ConflictResolution `DefMI != nullptr'. It attempts to join
live intervals for two adjacent instructions and erase the copy:

%2:vreg_256 = COPY %1
%3:vreg_256 = COPY killed %1

The LI needs to be adjusted to kill subrange for the erased instruction
and extend the subrange of the original def. That was done for the main
interval only but not for the subrange. As a result subrange had a VNI
pointing to the erased slot resulting in the above failure.

Diff Detail

Repository
rL LLVM

Event Timeline

rampitec created this revision.May 20 2019, 2:41 PM
arsenm added inline comments.May 20 2019, 2:46 PM
test/CodeGen/AMDGPU/coalescer-subranges-prune-kill-copy.mir
12 ↗(On Diff #200361)

Can you reduce this more? You shouldn't need all of these blocks

rampitec marked an inline comment as done.May 20 2019, 3:06 PM
rampitec added inline comments.
test/CodeGen/AMDGPU/coalescer-subranges-prune-kill-copy.mir
12 ↗(On Diff #200361)

I do not see how. This is bugpoint reduced on the IR level, and then bugpoint does not work with MIR yet. A manual reduce will be a nightmare.

arsenm added inline comments.May 20 2019, 3:13 PM
test/CodeGen/AMDGPU/coalescer-subranges-prune-kill-copy.mir
12 ↗(On Diff #200361)

Yes, I've done it many times. It's not that bad. It's fairly easy to eliminate the blocks, and then slows down inside the blocks

rampitec updated this revision to Diff 200379.May 20 2019, 5:22 PM
rampitec marked 2 inline comments as done.
rampitec edited the summary of this revision. (Show Details)

Reduced test.

I managed to reduce the test a bit more: https://ghostbin.com/paste/72n9f

I think this is correct

rampitec updated this revision to Diff 200538.May 21 2019, 10:44 AM

Use test further reduced by Matt.

arsenm accepted this revision.May 21 2019, 11:37 AM

LGTM

This revision is now accepted and ready to land.May 21 2019, 11:37 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2019, 12:31 PM