Add new pass to lower operations from the shape to the std dialect.
The conversion applies only to the size_to_index and to the index_to_size
operations.
Patterns will be added as needed.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/Conversion/ShapeToStandard/ShapeToStandard.cpp | ||
---|---|---|
22 | nit: no need for the empty line | |
23 | this class is not really needed. You can just move addLegalDialect<scf::SCFDialect, StandardOpsDialect>(); together with addLegalOp below. | |
32 | nit: no need for the empty line | |
35 | nit: no need for the empty line | |
50 | nit: no need for the empty line | |
53 | and here | |
64 | and even here | |
74 | nit: and here | |
79 | maybe just inline it in addConversion? | |
81 | why do you need to initialize it with nullptr? | |
100 | merge this with the first addLegalOp call ? | |
121 | you have two patterns here, but one test. | |
mlir/test/Conversion/ShapeToStandard/shape-to-standard.mlir | ||
10 | I don't think this comment brings a lot. Maybe smth like: // CHECK-LABEL: @size_to_index func @size_to_index(%size : !shape.size) -> !shape.size { return %size : !shape.size } // CHECK-SAME: ([[SIZE:%.*]]: index) // CHECK: return [[SIZE]] : index |
Thanks
mlir/lib/Conversion/ShapeToStandard/ShapeToStandard.cpp | ||
---|---|---|
81 | It's not really needed here, rather a habit that prevents uninitialised pointers in general. | |
100 | Good eye, thanks | |
121 | It's actually worse, I had no test for any of them | |
mlir/test/Conversion/ShapeToStandard/shape-to-standard.mlir | ||
10 | Updated the test cases and comments. Hope this makes it clear. |
mlir/lib/Conversion/ShapeToStandard/ShapeToStandard.cpp | ||
---|---|---|
62 | since you're copying ctx ptr into the lambda, you probably don't need it here. |
mlir/include/mlir/Conversion/ShapeToStandard/ShapeToStandard.h | ||
---|---|---|
12 | Can you trim this header? It doesn't seem necessary. | |
mlir/lib/Conversion/ShapeToStandard/ShapeToStandard.cpp | ||
17 | using namespace mlir; | |
93 | This isn't llvm style, please use explicit resolution: mlir:: populateShapeToStandardConversionPatterns Same below. |
Can you trim this header? It doesn't seem necessary.