This is an archive of the discontinued LLVM Phabricator instance.

[ModuloSchedule] Fix experimental modulo expansion for data loop carried dependencies.
ClosedPublic

Authored by ThomasRaoux on Oct 28 2019, 4:36 PM.

Details

Summary

The new experimental expansion has a problem when a value has a data dependency with am instruction from a previous stage. This is due to the way we peel out the kernel.
To fix that I'm changing the way we peel out the kernel. We now peel the kernel NumberStage - 1 times. The code would be correct at this point if we didn't have to handle cases where the loop iteration is smaller than the number of stages. To handle this case we move instructions between different epilogues based on their stage and remap the PHI instructions correctly.

Diff Detail

Event Timeline

ThomasRaoux created this revision.Oct 28 2019, 4:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 28 2019, 4:36 PM

Thanks Thomas!

llvm/include/llvm/CodeGen/ModuloSchedule.h
305

Please add a docstring

328

Please follow the style of the file: filterInstructions().

331

same here. "moveStageInBB" is a little ambiguous; can I suggest something like "moveStageBetweenBlocks"?

llvm/lib/CodeGen/ModuloSchedule.cpp
10

incorrect include here. Please make sure this builds.

1658

nit: remove brackets

1706

s/there/their/

llvm/test/CodeGen/Hexagon/swp-conv3x3-nested.ll
2

Was this xfail deliberately removed?

ThomasRaoux marked 7 inline comments as done.Nov 6 2019, 2:56 PM
ThomasRaoux added inline comments.
llvm/test/CodeGen/Hexagon/swp-conv3x3-nested.ll
2

Yes it is removed on purpose as with this change this program pipelines in 13 bundles as the test originally expect. Looking at it the code seems correct to me.

jmolloy accepted this revision.Nov 6 2019, 3:13 PM
This revision is now accepted and ready to land.Nov 6 2019, 3:13 PM
This revision was automatically updated to reflect the committed changes.