This is an archive of the discontinued LLVM Phabricator instance.

[MLIR][Affine][Analysis] Merge FAC and FACV
ClosedPublic

Authored by Groverkss on Mar 25 2022, 6:04 AM.

Details

Summary

With the introduction of IntegerPolyhedron and IntegerRelation in Presburger
directory, the purpose of FlatAffineConstraints becomes redundant. For users
requiring Presburger arithmetic without IR information, Presburger library can
directly be used. For users requiring IR information,
FlatAffineValueConstraints can be used.

This patch merges FAC and FACV to remove redundancy of FAC.

Diff Detail

Event Timeline

Groverkss created this revision.Mar 25 2022, 6:04 AM
Herald added a project: Restricted Project. · View Herald Transcript
Groverkss requested review of this revision.Mar 25 2022, 6:04 AM
arjunp added inline comments.Mar 27 2022, 9:41 AM
mlir/include/mlir/Dialect/Affine/Analysis/AffineStructures.h
36
mlir/lib/Dialect/Affine/Analysis/AffineStructures.cpp
40–46

I think these should be IntegerPolyhedron

Groverkss updated this revision to Diff 418558.Mar 28 2022, 5:41 AM
Groverkss marked 2 inline comments as done.
  • Address Arjun's comments
bondhugula added inline comments.Apr 5 2022, 12:20 AM
mlir/include/mlir/Dialect/Affine/Analysis/AffineStructures.h
37–58

Is this information being deleted already captured/subsumed elsewhere or is this being lost?

mlir/lib/Dialect/Affine/Analysis/AffineStructures.cpp
296–297

Looks like there is an API behavior difference here. An insertId for a 0 count should be valid and supported for completeness and to be intuitive. We shouldn't be using an extra check (for num == 0) at the user site. Can you update the insertId on IntegerPolyhedron to drop this check. (It's valid to say you are insert 0 things at pos -- so returning an absolute position is valid.)

Groverkss updated this revision to Diff 420413.Apr 5 2022, 2:00 AM
Groverkss marked 2 inline comments as done.
  • Rebase
  • Address Uday's comments
mlir/include/mlir/Dialect/Affine/Analysis/AffineStructures.h
37–58

The information about identifiers is already documented in PresburgerSpace. The information about inequalities and equalities is documented in IntegerPolyhedron. The only information lost is about reservedInequalities, reservedEqualities, reservedCols, which we can add to IntegerPolyhedron if required.

mlir/lib/Dialect/Affine/Analysis/AffineStructures.cpp
296–297

IntegerPolyhedron::insertId already does it. I think I added this for values.insert but it seems to work regardless so removing this if condition.

bondhugula added inline comments.Apr 5 2022, 5:28 AM
mlir/include/mlir/Dialect/Affine/Analysis/AffineStructures.h
37–58

I think reserved constraints size is an internal detail - fine to omit it here.

This change is looking good to me - thanks!

arjunp accepted this revision.Apr 5 2022, 2:14 PM
This revision is now accepted and ready to land.Apr 5 2022, 2:14 PM
This revision was automatically updated to reflect the committed changes.