Page MenuHomePhabricator

[RegisterCoalescer] dst register's live interval needs to be updated when merging a src register in ToBeUpdated set
ClosedPublic

Authored by wmi on Dec 18 2018, 4:39 PM.

Details

Summary

This is to fix the bug in PR40061 related with https://reviews.llvm.org/rL339035.

In https://reviews.llvm.org/rL339035, live interval of source pseudo register in rematerialized copy is saved in ToBeUpdated set and its update is postponed.

In PR40061, %t2 = %t1 is rematerialized and %t1 is added into toBeUpdated set to postpone its live interval update. After the rematerialization, the live interval of %t1 is larger than necessary. Then %t1 is merged into %t3 and %t1 gets removed. After the merge, %t3 contains live interval larger than necessary. Because %t3 is not in toBeUpdated set, its live interval is not updated after register coalescing and it will break some assumption in regalloc.

The patch requires the live interval of destination register in a merge to be updated if the source register is in ToBeUpdated.

Diff Detail

Repository
rL LLVM

Event Timeline

wmi created this revision.Dec 18 2018, 4:39 PM
wmi updated this revision to Diff 178816.Dec 18 2018, 4:53 PM

A minor update to the comment.

I've verified that this patch fixes the original problem.

Looks mostly good to me.

test/CodeGen/X86/late-remat-update-2.mir
1 ↗(On Diff #178816)

Could you come up with checks that don't require asserts?

wmi updated this revision to Diff 179321.Dec 21 2018, 11:34 AM
wmi marked an inline comment as done.

Address Quentin's comment.

wmi added inline comments.Dec 21 2018, 11:35 AM
test/CodeGen/X86/late-remat-update-2.mir
1 ↗(On Diff #178816)

Done. Thanks for the suggestion!

Ping, any progress on this?

wmi added a comment.Jan 7 2019, 12:35 PM

friendly ping. Please take another look.

This revision is now accepted and ready to land.Jan 7 2019, 2:07 PM
This revision was automatically updated to reflect the committed changes.