Page MenuHomePhabricator

[MLIR][Affine] Allow affine-expr on RHS in IntegerSet
ClosedPublic

Authored by Groverkss on Jun 30 2022, 7:35 AM.

Details

Summary

Currently, the parser for IntegerSet, only allows constraints like:

affine-constraint ::= affine-expr `>=` `0`
                    | affine-expr `==` `0`

This form is sometimes unreadable and painful to use when writing unittests
for Presburger library and tests in general.

This patch extends the parser to allow affine constraints with affine-expr on
the RHS:

affine-constraint ::= affine-expr `>=` `affine-expr`
                    | affine-expr `==` `affine-expr`

The internal storage and printing of IntegerSet is still in the original format.

Diff Detail

Event Timeline

Groverkss created this revision.Jun 30 2022, 7:35 AM
Herald added a project: Restricted Project. · View Herald Transcript
Groverkss requested review of this revision.Jun 30 2022, 7:35 AM
Groverkss edited the summary of this revision. (Show Details)Jun 30 2022, 7:39 AM
bondhugula accepted this revision.Jul 1 2022, 7:27 PM

This would be a great improvement -- thanks! LGTM with some comments.

mlir/lib/Parser/AffineParser.cpp
600

You'll need to update the documentation files (*.md) as well. (See Affine.md.)

mlir/test/IR/affine-set.mlir
4

The name of this file doesn't reflect these specific test cases related to checking affine expressions on the RHS. You could just move the other existing test cases on affine integer sets all into this file.

12

Isn't it five hyphens?! You don't even have a split-input-file on top. It looks like these are all dead lines.

mlir/test/IR/invalid.mlir
297

Can you add a similar negative test case for (i == )>?

This revision is now accepted and ready to land.Jul 1 2022, 7:27 PM
Groverkss updated this revision to Diff 441896.Jul 2 2022, 11:31 AM
Groverkss marked 4 inline comments as done.

Address Uday's comments

mlir/lib/Parser/AffineParser.cpp
600

I think I already did this.

mlir/test/IR/affine-set.mlir
4

Most test cases for affine_set involve sets which are used in affine.if as well, so it is hard to move them without duplicating tests.

I have a patch, which will add more tests to this file later.

I have also added some more tests to check some corner cases for affine_set since affine_set has very less tests already.

bondhugula accepted this revision.Jul 2 2022, 6:10 PM
This revision was automatically updated to reflect the committed changes.