Page MenuHomePhabricator

[MLIR][affine] Prevent fusion when ops with memory effect free are present between producer and consumer

Authored by vinayaka-polymage on Feb 18 2021, 9:49 PM.



This commit fixes a bug in affine fusion pipeline where an
incorrect fusion is performed despite a dealloc op is present
between a producer and a consumer. This is done by creating a
node for dealloc op in the MDG.

Diff Detail

Event Timeline

vinayaka-polymage requested review of this revision.Feb 18 2021, 9:49 PM
dcaballe requested changes to this revision.Feb 19 2021, 10:49 AM

Hey Vinayaka!

Thank you so much for the fix! Some comments below.


This looks good to me! I think we have to be more conservative and also add any non-affine operation that produces or uses a memref. Otherwise, hasNonAffineUsersOnThePath won't return the expected output if we have a read, call or alloc-like op or any unknown operation that could potentially make the memref escape, right?


Would you mind adding also tests with a call op, a load op and an unknown op using the memref, if we don't have it already? Thanks!


I think we should check loads and stores also because we may fuse and do not remove the producer loop.

This revision now requires changes to proceed.Feb 19 2021, 10:49 AM is somehow related. I think the common solution is to represent these missing dependences in the MDG. We may have to coordinate the fixes.

bondhugula accepted this revision.Feb 20 2021, 8:30 AM



I feel those can be handled in a next patch. This doesn't fix everything but is catching writing and freeing users the right way.


But this patch isn't meant to fix those. It's only for the users which have a write side-effect or a free side-effect.

dcaballe accepted this revision.Feb 21 2021, 10:27 PM

If you could improve the lit test so that loads and stores are also checked, that should be all. Thanks!


Ok, fair enough!

This revision is now accepted and ready to land.Feb 21 2021, 10:27 PM

Thanks for the suggestions!
I will create a new patch with the above suggested changes and paste the link here.


That's correct, I had missed this, thanks! I have corrected this.

bondhugula accepted this revision.Feb 22 2021, 7:57 AM
dcaballe accepted this revision.Feb 22 2021, 9:39 AM

Thanks for addressing the comments!

This revision was landed with ongoing or failed builds.Feb 22 2021, 9:51 AM
This revision was automatically updated to reflect the committed changes.

Suggested additional checks and test-cases submitted at