This is an archive of the discontinued LLVM Phabricator instance.

Remove unused stackslots generated in reg spills
ClosedPublic

Authored by wmi on Jan 29 2016, 12:30 PM.

Details

Reviewers
qcolombet
Summary

The problem and testcase is described in https://llvm.org/bugs/show_bug.cgi?id=26374.

The cause of the problem is:
LiveRangeEdit::eliminateDeadDef is used to remove dead define instructions after rematerialization. To remove a VNI for a vreg from its LiveInterval, LiveIntervals::removeVRegDefAt is used. However, after non-PHI VNIs are all removed, PHI VNI are still left in the LiveInterval. Such unused vregs will be kept in RegsToSpill[] at the end of InlineSpiller::reMaterializeAll and spiller will allocate stackslot for them.

The fix is to get rid of unused reg by checking whether it has non-dbg reference instead of whether it has non-empty interval.

For the 1.c testcase in PR26374, the stack allocated for function GetSkipCostMB dropped from 408 to 344.

Diff Detail

Repository
rL LLVM

Event Timeline

wmi updated this revision to Diff 46403.Jan 29 2016, 12:30 PM
wmi retitled this revision from to Remove unused stackslots generated in reg spills.
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, thanm.
qcolombet accepted this revision.Feb 4 2016, 2:54 PM
qcolombet edited edge metadata.

Hi Wei,

LGTM with a few nitpicks:

  1. Add a comment explaining what the test case is testing at the beginning of the file.
  2. Change the name of the test case to something relevant, i.e., pr26374.ll is a bad name!

Cheers,
-Quentin

lib/CodeGen/InlineSpiller.cpp
990–995

I liked the previous comments.

test/CodeGen/X86/pr26374.ll
1 ↗(On Diff #46403)

Could you add the description of the related PR, so that we do not have to look at the web page when we want to know what this is about.

This revision is now accepted and ready to land.Feb 4 2016, 2:54 PM
wmi marked 2 inline comments as done.Feb 5 2016, 10:13 AM

Addressed Quentin's comment.

wmi updated this revision to Diff 47027.Feb 5 2016, 10:14 AM
wmi edited edge metadata.

Thanks Wei!

Q.

Eugene.Zelenko added a subscriber: Eugene.Zelenko.

Committed in r259895.