Page MenuHomePhabricator

Relax FuseTensorReshapeOpAsproducer identity mapping constraint

Authored by asaadaldien on Oct 5 2020, 8:55 PM.



Allow fusing linalg.tensor_reshape with its consumer when the consumer is applying a permutation map (e.g linalg.generic implementing a transpose)

Diff Detail

Event Timeline

asaadaldien created this revision.Oct 5 2020, 8:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 5 2020, 8:55 PM
asaadaldien requested review of this revision.Oct 5 2020, 8:55 PM
asaadaldien edited the summary of this revision. (Show Details)Oct 5 2020, 8:57 PM
asaadaldien added a reviewer: maheshkhanwalkar.
hanchung added inline comments.

I think isPermutation covers isIdentity, so maybe no need to check the first case, ie, useIndexMap.isIdentity() || ?


I think we actually just want to make sure the map is invertible, so the comment is a bit not accurate. The identity map or permutation map is guaranteed to be invertible, so you don't need to check if invMap is null.



Checked, isPermutation doesn't exclude identity.


The comment above implies consumer map fusedIndexMaps[consumerIdx] is invertible it's just phrased differently.

mravishankar requested changes to this revision.Oct 6 2020, 10:18 AM

Thanks Ahmed, just one minor comment.


Do we need to make a distinction for when it is asProducer and not. I think you can make it isPermutation always? Then you can maybe also add a test where the reshape is the consumer?

This revision now requires changes to proceed.Oct 6 2020, 10:18 AM
asaadaldien added a comment.EditedOct 6 2020, 12:52 PM

Thanks Ahmed, just one minor comment.

Thanks Ahmed, just one minor comment.

I need to look closely into FuseTensorReshapeOpAsConsumer to see why it isn't producing the correct indexing map for the permutation case. I can do this in a follow up diff.

NVM, that was a silly test bug :D, ignore my comment.

enable premutations for tensor_reshape as consumer

mravishankar accepted this revision.Oct 6 2020, 2:46 PM
This revision is now accepted and ready to land.Oct 6 2020, 2:46 PM