Page MenuHomePhabricator

[MLIR][Shape] Lower `shape.shape_eq` to `scf`
AcceptedPublic

Authored by frgossen on Thu, Jun 25, 1:58 AM.

Details

Summary

Lower shape.shape_eq to the scf (and std) dialect.

Depends On D82529

Diff Detail

Event Timeline

frgossen created this revision.Thu, Jun 25, 1:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Jun 25, 1:58 AM
pifon2a added inline comments.Thu, Jun 25, 3:55 AM
mlir/lib/Conversion/ShapeToSCF/ShapeToSCF.cpp
42
if (!operandTy.isa<ShapeType>())
      return failure();
74

nit: replace auto with Value or Type where possible.

mlir/test/Conversion/ShapeToSCF/shape-to-scf.mlir
44

i bet you don't need most of these -DAGs.

pifon2a accepted this revision.Thu, Jun 25, 3:56 AM
This revision is now accepted and ready to land.Thu, Jun 25, 3:56 AM
frgossen marked an inline comment as done.Thu, Jun 25, 4:04 AM

Thanks

mlir/test/Conversion/ShapeToSCF/shape-to-scf.mlir
44

I like them because they make it clear where order matters and where it doesn't.
Should I change this?

frgossen updated this revision to Diff 273308.Thu, Jun 25, 4:52 AM
frgossen marked 2 inline comments as done.

Address comments

pifon2a accepted this revision.Thu, Jun 25, 5:37 AM
pifon2a added inline comments.
mlir/test/Conversion/ShapeToSCF/shape-to-scf.mlir
44

yes, please. It shows what deterministic and what not.

frgossen updated this revision to Diff 273376.Thu, Jun 25, 8:20 AM
frgossen marked 2 inline comments as done.

Remove CHECK-DAGs

frgossen updated this revision to Diff 273392.Thu, Jun 25, 8:52 AM

Remove CHECK-DAGs

herhut accepted this revision.Thu, Jun 25, 12:20 PM
herhut added inline comments.
mlir/lib/Conversion/ShapeToSCF/ShapeToSCF.cpp
45

nit: size -> shape?

63

A shame we cannot model early abort in SCF. This is ok for now, though. If we get to the point that shape comparisons are performance critical, we got quite far :)

rriddle added inline comments.Thu, Jun 25, 12:26 PM
mlir/lib/Conversion/ShapeToSCF/ShapeToSCF.cpp
45

Please add a bigger comment detailing how this lowering works. Having a before/after code sample is also generally acceptable.

frgossen updated this revision to Diff 273658.Fri, Jun 26, 4:08 AM
frgossen marked 3 inline comments as done.

Add documentation

frgossen retitled this revision from [MLIR][Shape] Lower `shape.eq` to `scf` to [MLIR][Shape] Lower `shape.shape_eq` to `scf`.Mon, Jun 29, 7:03 AM
frgossen edited the summary of this revision. (Show Details)