This is an archive of the discontinued LLVM Phabricator instance.

Fix live interval update of Dead PHI for r265547
ClosedPublic

Authored by wmi on Apr 9 2016, 11:01 AM.

Details

Reviewers
qcolombet
Summary

Before r265547, vreg defined by PHI VNI cannot be rematerialized because the def of PHI VNI has no instruction associated with it.

Now we use original def of vreg for rematerialization, so vreg defined by PHI VNI can be rematerialized. However, for rematerialization happen in splitting phase, live range update in splitting didn't update live range correctly if a PHI VNI becomes Dead after rematerialization. It will extend live range for Dead PHI, which may lead to some inconsistent state.

The fix is: move rewriteAssigned (which will extend live range according to real vreg uses) before extendPHIKillRanges. Then at the beginning of extendPHIKillRanges, if the segment associated with PHI->def is not extended, we know this is a Dead PHI. We will remove segment for Dead PHI, and skip live range extension for such PHI.

The bug could have been caught earlier if I run tests with -verify-regalloc, which I found quite powerful to catch inconsistent state in an early stage. After the fix, I run spec2006 and llvm testsuite with -verify-regalloc successfully, which provides me some confidence about correctness.

Diff Detail

Repository
rL LLVM

Event Timeline

wmi updated this revision to Diff 53141.Apr 9 2016, 11:01 AM
wmi retitled this revision from to Fix live interval update of Dead PHI for r265547.
wmi updated this object.
wmi added a reviewer: qcolombet.
wmi set the repository for this revision to rL LLVM.
wmi added subscribers: llvm-commits, davidxl.
wmi added a comment.Apr 9 2016, 11:07 AM

Forget to mention. The bugzilla entry is here: https://llvm.org/bugs/show_bug.cgi?id=27275

qcolombet accepted this revision.Apr 11 2016, 1:10 PM
qcolombet edited edge metadata.

Hi Wei,

LGTM modulo maybe a rephrase of the comment on the test cases.

Cheers,
-Quention

test/CodeGen/ARM/interval-update-remat.ll
3

What about:
When enabling remat for vreg defined by PHIs, make sure the update of the live range removes dead phi. Otherwise, we may end up with PHIs with incorrect operands and that will trigger assertions or verifier failures in later passes.

test/CodeGen/X86/interval-update-remat.ll
157

Run opt -instnamer to get rid of the %[0-9]+ variables.

This revision is now accepted and ready to land.Apr 11 2016, 1:10 PM
wmi updated this revision to Diff 53334.Apr 11 2016, 3:35 PM
wmi edited edge metadata.

Address Quentin's comments.

Eugene.Zelenko added a subscriber: Eugene.Zelenko.

Settled in r266162.