Skip to content

[Relay] Improve the "clip" op optimization in simplify expr pass#15068

Merged
masahi merged 7 commits intoapache:mainfrom
kfeng123:improve_simplify_expr
Jun 15, 2023
Merged

[Relay] Improve the "clip" op optimization in simplify expr pass#15068
masahi merged 7 commits intoapache:mainfrom
kfeng123:improve_simplify_expr

Conversation

@kfeng123
Copy link
Copy Markdown
Contributor

@kfeng123 kfeng123 commented Jun 9, 2023

This PR improves the optimization of clip in simplify expr pass.
Previously, there is a SimplifyCastClip class, it deals with the pattern "cast->clip": it checks if the a_min/a_max values of clip equals the min/max values of the data type of cast OP.
This PR improve SimplifyCastClip in 2 aspects:

  1. In the original code, it is restricted that the OP before "clip" is a "cast" OP. This restriction is not necessary, and is relaxed.
  2. In the original code, CheckDataTypeMaxMinValue checks if the a_min/a_max values of clip equals the min/max values of dtype. I relax this to check "a_max >= dtype.max and a_min <= dtype.min". Also, I add the case of bfloat16 in CheckDataTypeMaxMinValue.

Since the new optimization of "clip" does not rely on "cast" op, the name is changed to SimplifyClip.

@tvm-bot
Copy link
Copy Markdown
Collaborator

tvm-bot commented Jun 9, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

…is is to avoid destroying the structure required by LegalizeQnnOpForDnnl; 2. code reformatted.
@kfeng123
Copy link
Copy Markdown
Contributor Author

cc @ibsidorenko

@kfeng123 kfeng123 changed the title [Relay] Improve the optimization of clip in simplify expr pass [Relay][Bugfix] Improve and fix bug of the "clip" op optimization in simplify expr pass Jun 12, 2023
@kfeng123 kfeng123 changed the title [Relay][Bugfix] Improve and fix bug of the "clip" op optimization in simplify expr pass [Relay][Bugfix] Improve and fix a bug of the "clip" op optimization in simplify expr pass Jun 12, 2023
@kfeng123 kfeng123 changed the title [Relay][Bugfix] Improve and fix a bug of the "clip" op optimization in simplify expr pass [Relay] Improve the "clip" op optimization in simplify expr pass Jun 12, 2023
Comment thread src/relay/transforms/simplify_expr.cc Outdated
const ClipAttrs* clip_attrs = clip_node->attrs.as<ClipAttrs>();

// TODO(kfeng123): For now, the arg of "clip" is forced to not be "qnn.requantize" and
// "ann.add". This is to avoid destroying the structure required by LegalizeQnnOpForDnnl
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"qnn.add"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Typo fixed.

@kfeng123 kfeng123 requested a review from ibsidorenko June 14, 2023 09:38
Copy link
Copy Markdown
Contributor

@ibsidorenko ibsidorenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, what is motivation for this change? New case came from the real life DNN model... or this is required for some kind of synthetic test?

@kfeng123
Copy link
Copy Markdown
Contributor Author

By the way, what is motivation for this change? New case came from the real life DNN model... or this is required for some kind of synthetic test?

The motivation is just to improve simplify_expr pass. Did not come from a real model or a test case...

Nevertheless, this improvement may be practical after I develop further improvements. (I plan to give a series of PR to improve simplify_expr and related things shortly. There is still a lot of room for improvement in simplify_expr.)

Suppose we already have an improved optimization for "cast" op (although we do not have for now). Consider the following model fragment:

%input // uint8 
%0 = cast(%input, "float32") // float32
%1 = transpose(%0, axes = [1,0]) // float32
%2 = clip(%0, 0f, 255f) // float32

Note that the “cast” and "transpose" are commutative, and a lazy cast make transpose exected in uint8, which is faster:

%input // uint8 
%0 = transpose(%input, axes = [1,0]) // uint8
%1 = cast(%0, "float32") // float32
%2 = clip(%0, 0f, 255f) // float32

Next, note that the “cast” and "clip" are commutative, which leads to


%input // uint8 
%0 = transpose(%input, axes = [1,0]) // uint8
%1 = clip(%0, 0f, 255f) // uint8
%2 = cast(%0, "float32") // float32

For now, we can use the pass in this PR to remove clip. (Note that the original pass can not remove the clip in this case!)

@masahi masahi merged commit 62a5e7a into apache:main Jun 15, 2023
@kfeng123 kfeng123 deleted the improve_simplify_expr branch June 21, 2023 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants