Details
- Reviewers
nicolasvasilache rriddle
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/include/mlir/Dialect/MemRef/Transforms/Passes.h | ||
---|---|---|
113 | This should be camelCase. | |
121 | Drop the llvm here. | |
mlir/include/mlir/Dialect/MemRef/Transforms/Passes.td | ||
177 ↗ | (On Diff #419674) | Please keep these passes sorted. |
177 ↗ | (On Diff #419674) | This feels like it needs a lot more documentation. It isn't clear from reading this definition what the "printing" is supposed to entail. |
mlir/lib/Dialect/MemRef/Transforms/EmbedMemRefPrints.cpp | ||
33 | The drop directly to LLVM feels off for this level of abstraction, why is this necessary? | |
213 | This doesn't look great, I don't see why MemRef should be depending on SCF and Linalg here. |
mlir/lib/Dialect/MemRef/Transforms/EmbedMemRefPrints.cpp | ||
---|---|---|
33 | This is because I am adding global string constants and printing them. Is there any other way to print a string in MLIR right now? | |
213 | Can't we depend on other dialects in transformations at least? Just wondering, I removed Linalg and SCF dependency. |
mlir/include/mlir/Dialect/MemRef/Transforms/Passes.h | ||
---|---|---|
122 | I'm concerned about C++ injection in passes: it breaks reproducibility. |
mlir/include/mlir/Dialect/MemRef/Transforms/Passes.h | ||
---|---|---|
122 | Fair enough. It can be fixed by:
mlir::memref::embedMemRefPrints(ExtractValuesFunc extract_values_func) and creating a test pass that exercises it with the default extract_values_func. After that every user, who wants to print memref content will have to create a pass and call embedMemRefPrints inside runOnOperation with the filter they want. WDYT? |
mlir/include/mlir/Dialect/MemRef/Transforms/Passes.h | ||
---|---|---|
122 | utility function + test pass it the common pattern AFAIK |
This should be camelCase.