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 | ||
|---|---|---|
| 61 | since you're copying ctx ptr into the lambda, you probably don't need it here. | |
| mlir/include/mlir/Conversion/ShapeToStandard/ShapeToStandard.h | ||
|---|---|---|
| 13 | Can you trim this header? It doesn't seem necessary. | |
| mlir/lib/Conversion/ShapeToStandard/ShapeToStandard.cpp | ||
| 18 | using namespace mlir; | |
| 94 | This isn't llvm style, please use explicit resolution: mlir:: populateShapeToStandardConversionPatterns Same below. | |
Can you trim this header? It doesn't seem necessary.