I would like to remove the canonicalizer
DimOfShapedTypeOpInterface from the
memref.dim op, finding a place to relocate this to as part of a pass. In conjunction with the overall problems with the current placement of the
memref.dim op, I don’t think this this qualifies as a canonicalization, acting more as a lowering that should be optional to apply (and controllable as to when).
Between the various moves and refactors, I haven’t done enough archaeology to determine when this was added and for whom.
What this pattern does:
- If the producer does not implement
InferShapedTypeOpInterfaceor it is not for a constant index, fail.
- Invokes the op’s
reifyReturnTypeShapesto produce a
tensor<?xindex>based on the op’s implementation of a shape transfer function (which can produce arbitrary IR).
- Emit a
tensor.extract_elementto get the single dimension of interest.
The presence of this as a canonicalization pattern was recently surprising to me because we had correctly placed
memref.dim ops in our pipeline, which at a point, had producers that were high level ops implementing
InferShapedTypeOpInterface. Upon canonicalizing, what was a simple
memref.dim was expanded into a non-trivial sequence of
shape.shape_of, other shape dialect ops operating on index tensors, and
tensor.extract_element. There are auxiliary passes in the shape dialect for simplifying some of these things, but I found cases where they duel with this canonicalization in non-obvious ways dependent on phase ordering.
Our lowering pipeline actually rarely needs to resolve shape transfer functions in this way, exercising a direct lowering to shape-carrying low level ops for dynamic tensor-ops that have a representation. The facility is still useful for custom ops and other things, but we would want to use it explicitly in a controlled fashion vs treating as an automatic canonicalization.
I also just feel that this specific pattern has a smell to it of not actually being a canonicalization but a lowering (i.e. it completely changes the level of abstraction, expands to ops from multiple dialects, does not clearly produce a form that I would identify as “canonical”, etc).
Without knowing how this is used, I don’t have a strong opinion on where it should go instead (presumably it is load bearing for someone). Possibly in
ShapeToStandard (advantage: as part of dialect conversion, it could only be converted if the overall pattern application produces a legal lowering) or possibly in
I don’t want to break anyone’s flows and will work to accommodate, but as far as I can see, there are no in-tree live uses that depend on this and if no one speaks up, I think we should delete it outright. It will be necessary to untangle this in order to rehome/split the
memref.dim op anyway, which should not be carrying this baggage, imo.
Thanks - and sorry if this is already part of a plan somewhere or has an implication I’m not seeing. Feedback welcome.