[GraphOptimizer] Reshape sinking

[aturetsk]

While Reshape nodes are noops and do not directly contribute to model execution time, having them in a graph might prevent other optimizations/transformations from happening. So, sinking Reshapes (similar to Transposes) and merging them might potentially lead to better optimized graph.

One particular use case I encountered is coming from OptimizeTransposeIntoReshape optimization (#2630). There was a Transpose in a graph which Glow wasn't able to optimize out in a single glow::optimize run, so it sank till a specific point and then became a Reshape. On the next glow::optimize iteration, while conditions for further Transpose sinking were satisfied, it didn't happen because it was a Reshape now, not Transpose. Having Reshape sinking logic along with Transpose sinking should help to resolve such cases.

So, does it make sense to have Reshape sinking transformations in GraphOptimizer?

Also, I've noticed there is a transformation in Glow which is opposite to Reshape sinking - Reshape hoisting over Clip (#4074). @jfix71, could you give any comment on why we would do that?

[aturetsk]

cc: @vuzelac-cadence, @opti-mix

[jfix71]

Sure, I think it makes sense as a way to try to enable more optimizations, though I also wonder if sinking them may also prevent optimizations as well. It may make sense to add at the end of the pipeline so that the first optimize call sees the graph without them sunk, and the next optimize call seems them sunk and so may have new opportunities?

There was a Transpose in a graph which Glow wasn't able to optimize out in a single glow::optimize run, so it sank till a specific point and then became a Reshape.

Curious if you have an idea for why it wasn't able to continue being sunk on the first optimize run.

Also, I've noticed there is a transformation in Glow which is opposite to Reshape sinking - Reshape hoisting over Clip (#4074). @jfix71, could you give any comment on why we would do that?

I believe this was because we saw a pattern like Clip -> Reshape -> Clip, and so the opt would sink the clip and eliminate it. If we had reshape sinking below Clip it would accomplish the same thing.

[jfix71]

@aturetsk I actually just found a couple places I needed rescale sinking, which are being added as part of #5257. Not sure if you were thinking of having a dedicated relu sinking pass, but I added them to SinkCode.

[aturetsk]

@jfix71, thanks for feedback! I'll submit some MRs with Reshape sinking logic in the near future.

Curious if you have an idea for why it wasn't able to continue being sunk on the first optimize run.

There is a dependency between Transpose sinking and constant folding passes. We can sink Transpose through a binary arithmetic operator, but only if both inputs are Transposes or one of the inputs is a Constant/Splat. However, we could have a binary arithmetic node with one input being Tile + Constant. Current Transpose sinking logic can't handle that. However, after constant folding, Tile + Constant becomes just a Constant, so further sinking can be done. Probably having another sink pass between constant folding and OptimizeTransposeIntoReshape passes would resolve this. Or moving constant folding earlier in the pipeline. Though I did not investigate how it'd affect other transformations.