This is an archive of the discontinued LLVM Phabricator instance.

compile time improvement for MachineSink
ClosedPublic

Authored by hulx2000 on May 12 2015, 2:05 PM.

Details

Summary

Currently whenever we sink any instruction, we do clearKillFlags for every use of every use operand for that instruction, apparently there are a lot of duplication, therefore compile time penalties.

This patch collect all the interested registers first, do clearKillFlags for it all together at once at the end, so we only need to do clearKillFlags once for one register, duplication is avoided.

The speedup for one of our testcase is 10x

Suggestions are welcome.

Diff Detail

Event Timeline

hulx2000 retitled this revision from to compile time improvement for MachineSink.
hulx2000 updated this object.
hulx2000 edited the test plan for this revision. (Show Details)
hulx2000 set the repository for this revision to rL LLVM.
hulx2000 added a subscriber: Unknown Object (MLST).May 12 2015, 2:11 PM
hulx2000 updated this object.
hulx2000 removed rL LLVM as the repository for this revision.
hulx2000 set the repository for this revision to rL LLVM.
hulx2000 updated this object.
dberlin added inline comments.
lib/CodeGen/MachineSink.cpp
300
for (auto Reg : RegsToClearKillFlags)
  MRI->clearKillFlags(Reg);

RegsToClearKillFlags.clear()
hiraditya added inline comments.
lib/CodeGen/MachineSink.cpp
175

In line: 154, we check that SrcReg has only one non-debug-use. So I'm wondering why we need to clear kill flags in the entire function when there will be only one use of SrcReg i.e., in MI which is erased anyway.

772–773

Typo: `registers'

fix the typo I forgot earlier.

rebase the latest tip now just in case.

hulx2000 removed rL LLVM as the repository for this revision.

recover last change by last patch, now this is the final version

MatzeB accepted this revision.May 15 2015, 4:28 PM
MatzeB added a reviewer: MatzeB.
MatzeB added a subscriber: MatzeB.

LGTM with the following nitpick adressed:

lib/CodeGen/MachineSink.cpp
773–774

No need to state the fact that the flags may be incorrect a second time. I think it is enough to use something simple as:

if(MO.isReg() && MO.isUse())

RegsToClearKillFlags.set(MO.getReg()); // Remember to clear kill flags later.
This revision is now accepted and ready to land.May 15 2015, 4:28 PM

Do you have commit access or should I commit this for you?

Yes, please commit it for me, since you are on it, could you please fix the comment for me too.

Thanks a lot.

lib/CodeGen/MachineSink.cpp
175

I will remove that, this is not the timing consuming part anyway.

300

will do that, thanks.

This revision was automatically updated to reflect the committed changes.