This is an archive of the discontinued LLVM Phabricator instance.

[GreedyRA] Fix removeBackCopies to account empty copies
ClosedPublic

Authored by skatkov on Apr 21 2021, 4:46 AM.

Details

Summary

RegAssignMap cannot hold empty interval, so do not set stop
to kill value if it produces empty interval.

This can happen if we remove back-copy and right before that we have another
back-copy.

For example, for parent %0 we can get
%1 = COPY %0
%2 = COPY %0
while we removing %2 we cannot set kill for %1 due to its empty.

Diff Detail

Event Timeline

skatkov created this revision.Apr 21 2021, 4:46 AM
skatkov requested review of this revision.Apr 21 2021, 4:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2021, 4:46 AM
skatkov edited the summary of this revision. (Show Details)Apr 22 2021, 1:22 AM

This seems to assume that producing a dead copy is expected behavior. Are you sure that the input form you describe is itself expected? It seems a bit suspect.

llvm/test/CodeGen/X86/statepoint-invoke-ra-remove-back-copies.mir
12

Any chance this can be further reduced? This looks really fragile.

empty copy is introduced by D100748 when we skip RegAssign.insert.
When another copy is hoisted after that we can get these two copies.

llvm/test/CodeGen/X86/statepoint-invoke-ra-remove-back-copies.mir
12

I did my best to reduce it using bugpoint, this is minimal version.

reames accepted this revision.May 3 2021, 6:32 PM

LGTM.

I'm a little hesitant about the combination of this and the previous patch with the empty copy, but honestly, I don't have a better suggestion. I'm okay going with this for now, but won't be surprised if we find ourselves revisiting this decision in the future.

This revision is now accepted and ready to land.May 3 2021, 6:32 PM