Page MenuHomePhabricator

[MLIR][STD] Fold trunci (zexti).
ClosedPublic

Authored by ergawy on Mar 27 2021, 3:32 AM.

Details

Summary

This patch folds the following pattern:

%arg0 = ...
%0 = zexti %arg0 : i1 to i8
%1 = trunci %0 : i8 to i1

into just %arg0.

Diff Detail

Event Timeline

ergawy created this revision.Mar 27 2021, 3:32 AM
ergawy requested review of this revision.Mar 27 2021, 3:32 AM

I came across this pattern while working on detensoring in linalg-on-tensors for IREE. Let me know if there are any reasons why this might be a bad idea.

mehdi_amini accepted this revision.Mar 27 2021, 9:42 AM

Should be valid with sexti as well right?

If the bit-widths of trunci's destination and source types do not match the bit-widths of zexti's source and destination types, respectively, no folding takes place.

Your folder does not do that actually. It is just the framework that does not perform such replacement for all folders.

(side note, you could write simply : If the type of trunci's destination does not match zexti's source types, no folding takes place. ; the intermediate type does not matter here)

mlir/lib/Dialect/StandardOps/IR/Ops.cpp
2185

Can you add a one line comment: trunci(zexti(a)) -> a

This revision is now accepted and ready to land.Mar 27 2021, 9:42 AM
mehdi_amini added inline comments.Mar 27 2021, 9:43 AM
mlir/test/Transforms/canonicalize.mlir
1095

This could be canonicalized to zexti %arg0 : i1 to i2 directly

ergawy updated this revision to Diff 333679.Mar 27 2021, 11:35 AM
ergawy marked an inline comment as done.
  • Handle review comments.
  • Rebase.

Your folder does not do that actually. It is just the framework that does not perform such replacement for all folders.
(side note, you could write simply : If the type of trunci's destination does not match zexti's source types, no folding takes place. ; the intermediate type does not matter here)

I removed this part of the commit message to describe the changes introduced more accurately.

mlir/test/Transforms/canonicalize.mlir
1095

Added a TODO. I will address this it in a follow up partch.

ergawy edited the summary of this revision. (Show Details)Mar 27 2021, 11:36 AM

Should be valid with sexti as well right?

Will also do in a follow up patch.

This revision was landed with ongoing or failed builds.Mar 27 2021, 11:40 AM
This revision was automatically updated to reflect the committed changes.