Page MenuHomePhabricator

[MLIR][Presburger] Move functionality from IntegerPolyhedron to IntegerRelation
ClosedPublic

Authored by Groverkss on Feb 28 2022, 3:18 AM.

Details

Summary

This patch moves all functionality from IntegerPolyhedron to IntegerRelation.
IntegerPolyhedron is now implemented as a relation with no domain. All existing
functionality is extended to work on relations.

This patch does not affect external users like FlatAffineConstraints as they
can still continue to use IntegerPolyhedron abstraction.

This patch is part of a series of patches to support relations in Presburger
library.

Diff Detail

Event Timeline

Groverkss created this revision.Feb 28 2022, 3:18 AM
Groverkss requested review of this revision.Feb 28 2022, 3:18 AM
arjunp added inline comments.Feb 28 2022, 9:26 AM
mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
9–11

Maybe add 1-2 lines to explain what that is.

186–197

Note that domain is minimized, then range.

410

write that it constructs with zero range and with SpaceKind = Set

529
533

what does "between dimensions" mean?

mlir/include/mlir/Analysis/Presburger/PWMAFunction.h
64–65

poly -> rel?

mlir/include/mlir/Dialect/Affine/Analysis/AffineStructures.h
182

We don't support IntegerRelation here right? This should stay as IntegerPolyhedron?

487

Again, do we support IntegerRelation here?

mlir/lib/Analysis/Presburger/IntegerRelation.cpp
56

getNumDimIds() should not be supported on relation spaces, should assert spaceKind == Set in that I feel, and provide a getNumDomainAndRangeIds()

60

ret and rel are a little too similar. Maybe call the return value result

68

also below.

Groverkss updated this revision to Diff 412017.Mar 1 2022, 2:17 AM
Groverkss marked 11 inline comments as done.
  • Address Arjun's comments
mlir/include/mlir/Dialect/Affine/Analysis/AffineStructures.h
182

This is a virtual function used internally in IntegerRelation. So, it is required to be IntegerRelation.

487

Same reasoning as above.

mlir/lib/Analysis/Presburger/IntegerRelation.cpp
56

I prefer getNumDimIds() over this since domain and range identifiers are both dimension identifiers.

arjunp requested changes to this revision.Mar 2 2022, 2:46 AM

LGTM, please fix minor comments below.

mlir/include/mlir/Analysis/Presburger/IntegerRelation.h
9–11

A relation

11

variables

534

Kinds of ids

This revision now requires changes to proceed.Mar 2 2022, 2:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2022, 2:46 AM
arjunp accepted this revision.Mar 2 2022, 2:46 AM

Oops, selected the wrong action.

This revision is now accepted and ready to land.Mar 2 2022, 2:46 AM
Groverkss updated this revision to Diff 412395.Mar 2 2022, 6:30 AM
Groverkss marked 3 inline comments as done.
  • Address Arjun's comments
  • Address Arjun's comments
This revision was landed with ongoing or failed builds.Mar 2 2022, 6:44 AM
This revision was automatically updated to reflect the committed changes.