Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| mlir/include/mlir/IR/Dialect.h | ||
|---|---|---|
| 46 | Do we need the bool result here? This is only used for name shadowing (IIRC), so do we have an actual need to allow unregistered operations to name shadow? Also, I would expect that if an unregistered op does have that trait, we would develop a proper mechanism (e.g. via a dialect fallback) to query if the op has that trait. WDYT? | |
| mlir/lib/IR/AsmPrinter.cpp | ||
| 2412 | ||
| mlir/lib/Parser/Parser.cpp | ||
| 1658–1660 | nit: Drop the llvm:: here. | |
| mlir/include/mlir/IR/Dialect.h | ||
|---|---|---|
| 46 | Yeah that's a good point! | |
| mlir/include/mlir/IR/Dialect.h | ||
|---|---|---|
| 46 | Could we return function_ref (or llvm::unique_function) instead? I imagine that a majority of the functions passed back are going to be function-pointer-like(i.e. cheap to construct/copy). | |
| mlir/lib/Parser/Parser.cpp | ||
| 930 | nit: Drop the llvm:: here. | |
| mlir/test/lib/Dialect/Test/TestDialect.cpp | ||
| 213 | nit: Add braces here. | |
| mlir/include/mlir/IR/Dialect.h | ||
|---|---|---|
| 42 | Oh I forgot to update the doc when I changed the API... | |
Address River's most recent comments
| mlir/lib/Parser/Parser.cpp | ||
|---|---|---|
| 1708 | Yes! | |
This one I struggle to read (perhaps as I keep on reading bookmark). Is isolated from above returned/required still?