Page MenuHomePhabricator

[mlir][normalize-memrefs] Non-normalizable operations with identity map layouts do not block normalization of the entire function

Authored by tungld on May 17 2022, 11:36 PM.



The current approach is convervative in which whenever there is a non-normalizable operations in a function will the function be labelled as non-normalizable. It means it requires that all operations must have MemRefsNormalizable trait.

This patch relaxes the requirement that if the memref map layouts of a non-normalizable operation are identity, this operation does not block the normalization of the other operations in the same function.

Diff Detail

Event Timeline

tungld created this revision.May 17 2022, 11:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2022, 11:36 PM
tungld requested review of this revision.May 17 2022, 11:36 PM
bondhugula added inline comments.May 18 2022, 9:55 AM

This isn't the right to check for an identity layout. Instead, use isIdentity() on the layout itself.


Likewise: here and below.


Is this the right test case? In this case, all users of %arg0 are already normalizable and %arg0 doesn't have an identity layout map. Am I missing something?

tungld updated this revision to Diff 430558.May 18 2022, 7:21 PM
  • Use isIdentity() on the layout itself
tungld marked 2 inline comments as done.May 18 2022, 7:28 PM
tungld added inline comments.

Done. Thanks!


Yes, it is. Without this patch, %arg0 will not be normalized and the whole function stays unchanged. That's because the pass detects that the op_nonnorm op does not have MemRefsNormalizable trait no matter the op's arguments' map layouts are identity.

tungld marked an inline comment as done.May 19 2022, 12:47 AM

@bondhugula Is the current change ok? or should I make any other change? Thanks!

@bondhugula @avarmapml @imaihal could you please review this patch again? Thanks!

bondhugula accepted this revision.Aug 18 2022, 7:23 PM

LGTM - thanks.


You can use allocOp.getType() to avoid using cast.


Use dyn_cast instead to avoid a repeated check.


Use dyn_cast.

This revision is now accepted and ready to land.Aug 18 2022, 7:23 PM