This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by frgossen on Jun 25 2020, 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.Jun 25 2020, 1:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 25 2020, 1:58 AM
pifon2a added inline comments.Jun 25 2020, 3:55 AM
mlir/lib/Conversion/ShapeToSCF/ShapeToSCF.cpp
40
if (!operandTy.isa<ShapeType>())
      return failure();
72

nit: replace auto with Value or Type where possible.

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

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

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

Thanks

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

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.Jun 25 2020, 4:52 AM
frgossen marked 2 inline comments as done.

Address comments

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

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

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

Remove CHECK-DAGs

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

Remove CHECK-DAGs

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

nit: size -> shape?

61

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.Jun 25 2020, 12:26 PM
mlir/lib/Conversion/ShapeToSCF/ShapeToSCF.cpp
43

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.Jun 26 2020, 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`.Jun 29 2020, 7:03 AM
frgossen edited the summary of this revision. (Show Details)
frgossen updated this revision to Diff 278467.Jul 16 2020, 7:16 AM

Update to extent tensor variant

This revision was automatically updated to reflect the committed changes.