Page MenuHomePhabricator

[ModuloSchedule] Fix a bug in experimental expander during prologue/epilogue stitching.
ClosedPublic

Authored by ThomasRaoux on Nov 13 2019, 4:33 PM.

Details

Summary

Fix two problems that popped up after my last patch. One is that the
stitching of prologue/epilogue can be wrong when reading a value from a
previous stage. Also changed how we duplicate phi instructions to avoid
generating extra phi that we delete later.

Diff Detail

Event Timeline

ThomasRaoux created this revision.Nov 13 2019, 4:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 13 2019, 4:33 PM
jmolloy added inline comments.Nov 14 2019, 12:53 AM
llvm/lib/CodeGen/ModuloSchedule.cpp
1671–1672

What's the rationale for this change? Is this just to avoid creating a phi that we destroy later?

If so, I'd prefer to keep the logic simple over optimal. We can always clean up dead nodes easily.

1792

The nesting is getting too deep here, let's move this into a helper function.

ThomasRaoux marked 2 inline comments as done.Nov 14 2019, 9:58 AM
ThomasRaoux added inline comments.
llvm/lib/CodeGen/ModuloSchedule.cpp
1671–1672

Yes the goal is to avoid inserting useless Phi. The problem is that when we have a large number of stages we can run into combinatorial explosion since we move the phi instructions block by block. I moved the logic in a lambda to make it easier to read.

@jmolloy, could you take one more look when you get a chance?

jmolloy accepted this revision.Nov 22 2019, 8:07 AM

LGTM, sorry for the delay.

This revision is now accepted and ready to land.Nov 22 2019, 8:07 AM

LGTM, sorry for the delay.

Thanks James!

This revision was automatically updated to reflect the committed changes.