add reshape handler #10627
add reshape handler #10627
Conversation
| if (shape_data == nullptr || shape_data->Data().size() == 0) { | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
Do you need to check that there's only one dim that is not == 1?
There was a problem hiding this comment.
Consider X->Transpose->Y->Reshape->Z, we check shape(X) == shape(Z), that is why I did not add another check for validating only 1 dim is not 1.
There was a problem hiding this comment.
Doesn't that assume the Reshape is always undo-ing the Transpose?
If more than one dim has a value of 1, the Reshape is not equivalent to a Transpose. Not sure if that occurs in a model, but it seems risky for that not to be explicitly validated.
There was a problem hiding this comment.
Right, we remove the 2 nodes only when they cancel each other out. In all other cases we keep them.
| static bool HandleReshape(HandlerArgs& args) { | ||
| // We check for a very specific case where Tranpose is replaced by Reshape | ||
| // for performance. For example Transpose(input {1, 1, 1, X}, perm{0, 3, 2, 1}) can be replaced by Reshape | ||
| // Reshape(input{1, 1, 1, X}, shape{1, X, 1, 1}) |
There was a problem hiding this comment.
Can this be generalized to handle {n, 1, 1, x} => {n, x, 1, 1}? Can be something in the future
| return false; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
std::count_if would make it a one-liner.
Description: Adding a handler for reshape node in transpose optimizer. This handler checks whether the Transpose->Reshape
combination can be removed.
Motivation and Context
Why is this change required? What problem does it solve?
This is a perf optimization. Trying to remove as many layout changing transposes as possible
If it fixes an open issue, please link to the issue here.