The documentation for the NormalizeMemRefs pass and the associated MemRefsNormalizable
traits was confusing and not on the website. This update clarifies the language
around the difference between a MemRef Type, an operation that accesses the value of
MemRef Type, and better documents the limitations of the current implementation.
This patch also includes some basic debugging information for the pass so people
might have a chance of figuring out why it doesn't work on their code.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/docs/Traits.md | ||
---|---|---|
254–262 | Hmm... this isn't entirely accurate. We may still need to tweak/improve the original thing. Ops like DeallocOp, CallOp, and ReturnOp also have this trait but they don't access the memref through an arbitrary affine subscript function. The trait is more about when you can normalize the memref operand on the op safely / meaningfully. |
Thanks for submitting this - esp. the detailed pass description. We should definitely improve the documentation since you didn't find it clear enough.
Added suggestion, thanks for attempting to clarify the original text, much appreciated.
mlir/include/mlir/Transforms/Passes.td | ||
---|---|---|
327 | "must" That is probably too strong, as operations can be handled directly in the pass. I would say what the flag does, namely it will let the pass assume that normalizing that operation is safe. | |
331 | "Currently...." maybe this is too strong a statement as we are adding support, and then we would have to correct this list. I would prefer something weaker. "Initial support includes...." | |
337 | Nit: all passes have to be conservative in general. I would just simply state the current limitation. Currently the pass is limited to handle functions where all ... and if you feel like adding this We may address this limitation if the need arises in the future. Fine either ways |
A few more minor comments.
mlir/docs/Traits.md | ||
---|---|---|
254 | access -> use? (access could imply dereferencing, which isn't the case with dealloc or call). | |
258 | it's -> its | |
mlir/include/mlir/IR/OpDefinition.h | ||
1215 | Likewise. | |
1216–1218 | Looks like this hasn't been updated. | |
mlir/include/mlir/Transforms/Passes.td | ||
317–325 | Thanks very much for this description. |
access -> use? (access could imply dereferencing, which isn't the case with dealloc or call).