Page MenuHomePhabricator

[Matrix] Update shape propagation to iterate until done.
ClosedPublic

Authored by fhahn on Dec 2 2019, 5:59 AM.

Details

Summary

This patch updates the shape propagation to iterate until no new shape
information is discovered.

As initial seed for the forward propagation, we use the matrix intrinsic
instructions. Both propagateShapeForward and propagateShapeBackward
return new work lists, with the instructions to be used for the next
iteration. When propagating forward, we record all instructions we added
new shape information for. When propagating backward, we record all
users of instructions we added new shape information for.

Diff Detail

Event Timeline

fhahn created this revision.Dec 2 2019, 5:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2019, 5:59 AM
Herald added a subscriber: hiraditya. · View Herald Transcript

Build result: FAILURE - Could not check out parent git hash "bc8e8995246e421e2c1b2f415685f8c86bf0aeb0". It was not found in the repository. Did you configure the "Parent Revision" in Phabricator properly? Trying to apply the patch to the master branch instead...

ERROR: arc patch failed with error code 1. Check build log for details.
Log files: console-log.txt, CMakeCache.txt

reames resigned from this revision.Dec 2 2019, 4:50 PM
LuoYuanke added inline comments.Dec 11 2019, 5:38 AM
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
486

Do we miss the original op ( matrix_multiply, matrix_transpose ...) in backward?

fhahn updated this revision to Diff 233634.Dec 12 2019, 9:04 AM

Update test checks after address computation simplification in earlier patches.

fhahn marked an inline comment as done.Dec 13 2019, 9:00 AM
fhahn added inline comments.
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
486

No, we add all instructions we update shape information for in propagateShapeForward, so multiply, transpose & co will be added there as well.

anemet accepted this revision.Dec 19 2019, 3:13 PM

LGTM.

llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
393

I think that this should take the WorkList as a parameter so that it's clear it's not pushing to the NewWorkList

407

WSize -> BeforeProcessingV or something like that

450

... we use their users ...

454

Should this exclude V?

This revision is now accepted and ready to land.Dec 19 2019, 3:13 PM
fhahn marked 4 inline comments as done.Jan 9 2020, 2:51 AM
This revision was automatically updated to reflect the committed changes.