Page MenuHomePhabricator

[mlir][math] Added constant folding for IPowI operation.
ClosedPublic

Authored by vzakhari on Jul 22 2022, 12:28 PM.

Diff Detail

Event Timeline

vzakhari created this revision.Jul 22 2022, 12:28 PM
vzakhari requested review of this revision.Jul 22 2022, 12:28 PM
vzakhari updated this revision to Diff 446949.Jul 22 2022, 12:42 PM
vzakhari updated this revision to Diff 449428.Aug 2 2022, 2:01 PM

rebase

Please review.

Mogball requested changes to this revision.Aug 11 2022, 10:03 AM
Mogball added a subscriber: Mogball.
Mogball added inline comments.
mlir/test/Dialect/Math/canonicalize_ipowi.mlir
440

why are these tests so complicated? why do you need memref in here?

This revision now requires changes to proceed.Aug 11 2022, 10:03 AM
vzakhari added inline comments.Aug 11 2022, 10:10 AM
mlir/test/Dialect/Math/canonicalize_ipowi.mlir
440

Thank you for looking into it, @Mogball!

Basically, I wanted to test multiple folding cases in one function, so I had to make use of the ipowi in some way (so that it does not get removed completely). I made the uses by storing the results into their own cells of the array passed as a pointer to the function. I guess I can just make the function return 11 results and get rid of the store operations. Or I can also split the function into 11 functions. Please let me know which of the options you prefer.

Mogball added inline comments.Aug 11 2022, 10:15 AM
mlir/test/Dialect/Math/canonicalize_ipowi.mlir
440

The second one.

The folder does look complicated. Do these tests cover each set of cases once?

vzakhari added inline comments.Aug 11 2022, 10:19 AM
mlir/test/Dialect/Math/canonicalize_ipowi.mlir
440

Yes, I tried to test each line of code at least once. For some of the cases I added more than one test, e.g. for positive powers I test negative and positive base values, whereas the folding code does not care about the base sign.

Mogball added inline comments.Aug 11 2022, 10:20 AM
mlir/test/Dialect/Math/canonicalize_ipowi.mlir
440

SG. Please split these up and document which case you are testing with each.

vzakhari updated this revision to Diff 452028.Aug 11 2022, 4:14 PM
Mogball accepted this revision.Aug 15 2022, 11:03 AM
This revision is now accepted and ready to land.Aug 15 2022, 11:03 AM
This revision was automatically updated to reflect the committed changes.