[Relay] Legalize pass #3672

anijain2305 · 2019-07-30T21:35:02Z

Relevant Issue - #3670

This pass transforms an expression to other expression.

This pass has many usecases

Replace a expr to another expr, if the other expr has faster performance.
For ASICs, we might want to modify the inputs to adapt to the HW support.
Alter op layout can work in conjunction with this pass.

The supporting usecase is the Intel i8 x i8 conv. Intel HW supports u8 x i8 conv
in HW. Using this pass, we can replace an i8 x i8 conv to a sequence of
operators where one of the operators is now u8 x i8 conv.

include/tvm/relay/op_attr_types.h

yzhliu · 2019-07-31T07:25:43Z

src/relay/pass/rewrite_op.cc

+// Returns the altered expression
+Expr OpRewriter(const Call &ref_call,
+                          const Array<Expr> &new_args,
+                          const NodeRef& ctx) {


align indent

yzhliu · 2019-07-31T07:26:06Z

src/relay/pass/rewrite_op.cc

+  if (fop_rewrite.count(op)) {
+    tvm::Array<tvm::Tensor> tinfos;
+    for (auto expr : ref_call->args) {
+      auto ttype = expr->type_as<TensorTypeNode>();


it could be TupleTypeNode

yzhliu · 2019-07-31T07:30:23Z

src/relay/pass/rewrite_op.cc

+  bool modified = false;
+  if (fop_rewrite.count(op)) {
+    tvm::Array<tvm::Tensor> tinfos;
+    for (auto expr : ref_call->args) {


Suggested change

for (auto expr : ref_call->args) {

for (auto& expr : ref_call->args) {

This pass transforms an expression to other expression. This pass has many usecases * Replace a expr to another expr, if the other expr has faster performance. * For ASICs, we might want to modify the inputs to adapt to the HW support. * Alter op layout can work in conjunction with this pass. The supporting usecase is the Intel i8 x i8 conv. Intel HW supports u8 x i8 conv in HW. Using this pass, we can replace an i8 x i8 conv to a sequence of operators where one of the operators is now u8 x i8 conv. This will also help automatic quantizaion performance.

tqchen · 2019-08-01T19:42:51Z

Let us debate on the name of the pass. Rewrite is a too generic. @FrozenGene @yzhliu @merrymercy @jroesch any thoughts?

anijain2305 · 2019-08-01T21:39:45Z

How about Strength Reduction? Not sure. But the idea is to replace one expr with other expr with the goal that it will make it faster. On the other hand, Strength Reduction might sound target agnostic, but this one is not.

FrozenGene · 2019-08-02T09:38:29Z

yeah, I also think Rewrite is too generic. Especially we have another api named as RewriteAnnotatedOps. Rewrite make us confuse these two apis at the first glance. Maybe we could name it ReweiteXXXOps like RewriteAnnotatedOps?

tqchen · 2019-08-02T16:11:30Z

Let us call it Legalize, as per LLVM terminology. https://wiki.aalto.fi/display/t1065450/LLVM+Legalization+and+Lowering

The first step when to designing API is to check conventional APIs and try to make things as consistent as possible.

anijain2305 · 2019-08-02T18:00:19Z

Thanks for the pointers for considering APIs. I have changed the name to Legalize.

Please review again.

FrozenGene · 2019-08-05T12:34:44Z

I will review the code tomorrow. Sorry for this. Because Monday is always very busy. :-(

FrozenGene

LGTM. Some comments of code should be handled.

FrozenGene · 2019-08-06T07:35:27Z

python/tvm/relay/transform.py

@@ -435,6 +435,21 @@ def AlterOpLayout():
    return _transform.AlterOpLayout()


+def Legalize():
+    """Rewrites a expression with another expression.


Maybe we should change Rewrites to Legalize

FrozenGene · 2019-08-06T07:38:01Z

tests/python/relay/test_pass_legalize.py

+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Test alter op layout pass"""


Test legalize pass

yzhliu

We can merge once @FrozenGene 's comments have been addressed.

icemelon · 2019-08-06T19:57:17Z

Should we move the expansion of batch_norm op from simplify inference to legalize pass?

anijain2305 · 2019-08-06T21:21:49Z

move the expansion of batch_norm op from simplify inference to legalize pass?

I like it. This is a target-independent pass though (which is fine, if everybody agrees to it).

Simplify_inference is anyways a vague name. From the name, it seems like something really big is happening inside :)

But, I would suggest doing it in a separate PR to avoid complicating this PR, as it will likely involve the discussion of when we should call this pass.

yzhliu · 2019-08-06T22:23:49Z

Thanks @anijain2305 @tqchen @FrozenGene @icemelon9

* [Relay] Rewrite pass. This pass transforms an expression to other expression. This pass has many usecases * Replace a expr to another expr, if the other expr has faster performance. * For ASICs, we might want to modify the inputs to adapt to the HW support. * Alter op layout can work in conjunction with this pass. The supporting usecase is the Intel i8 x i8 conv. Intel HW supports u8 x i8 conv in HW. Using this pass, we can replace an i8 x i8 conv to a sequence of operators where one of the operators is now u8 x i8 conv. This will also help automatic quantizaion performance. * Better API name. * Removing the conv2d legalization for x86. Will send a separate PR. * Test name changes. * Registering one funtion to register FTVMLegalize. * Better comments.

shoubhik reviewed Jul 30, 2019

View reviewed changes

include/tvm/relay/op_attr_types.h Show resolved Hide resolved

anijain2305 force-pushed the intel_int8 branch from b3244ca to 523bc84 Compare July 31, 2019 00:34

yzhliu reviewed Jul 31, 2019

View reviewed changes

anijain2305 force-pushed the intel_int8 branch 2 times, most recently from 793e109 to 214a333 Compare July 31, 2019 21:52

anijain2305 force-pushed the intel_int8 branch from 214a333 to 5f7b79c Compare July 31, 2019 22:21

tqchen added the status: api design review label Aug 1, 2019

tqchen mentioned this pull request Aug 2, 2019

[QNN] Requantize operator #3531

Merged

anijain2305 added 2 commits August 2, 2019 17:53

Better API name.

f91de7e

Removing the conv2d legalization for x86. Will send a separate PR.

bf11ee6

anijain2305 force-pushed the intel_int8 branch from e60f923 to bf11ee6 Compare August 2, 2019 17:56

Test name changes.

93a91e2

anijain2305 changed the title ~~[Relay] Rewrite pass.~~ [Relay] Legalize pass Aug 2, 2019

Registering one funtion to register FTVMLegalize.

b225821

tqchen assigned yzhliu and FrozenGene Aug 3, 2019

FrozenGene approved these changes Aug 6, 2019

View reviewed changes

yzhliu approved these changes Aug 6, 2019

View reviewed changes

Better comments.

c9d1980

yzhliu merged commit 79922bd into apache:master Aug 6, 2019

icemelon added status: accepted and removed status: api design review labels Aug 6, 2019

tqchen mentioned this pull request Nov 8, 2019

[RELEASE][DRAFT] TVM v0.6 Release candidate #4259

Closed

anijain2305 deleted the intel_int8 branch November 13, 2019 00:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Relay] Legalize pass #3672

[Relay] Legalize pass #3672

anijain2305 commented Jul 30, 2019

yzhliu Jul 31, 2019

yzhliu Jul 31, 2019

yzhliu Jul 31, 2019

tqchen commented Aug 1, 2019 •

edited

Loading

anijain2305 commented Aug 1, 2019

FrozenGene commented Aug 2, 2019

tqchen commented Aug 2, 2019 •

edited

Loading

anijain2305 commented Aug 2, 2019

FrozenGene commented Aug 5, 2019

FrozenGene left a comment

FrozenGene Aug 6, 2019

FrozenGene Aug 6, 2019

yzhliu left a comment

icemelon commented Aug 6, 2019

anijain2305 commented Aug 6, 2019

yzhliu commented Aug 6, 2019 •

edited

Loading

	for (auto expr : ref_call->args) {
	for (auto& expr : ref_call->args) {

[Relay] Legalize pass #3672

[Relay] Legalize pass #3672

Conversation

anijain2305 commented Jul 30, 2019

yzhliu Jul 31, 2019

Choose a reason for hiding this comment

yzhliu Jul 31, 2019

Choose a reason for hiding this comment

yzhliu Jul 31, 2019

Choose a reason for hiding this comment

tqchen commented Aug 1, 2019 • edited Loading

anijain2305 commented Aug 1, 2019

FrozenGene commented Aug 2, 2019

tqchen commented Aug 2, 2019 • edited Loading

anijain2305 commented Aug 2, 2019

FrozenGene commented Aug 5, 2019

FrozenGene left a comment

Choose a reason for hiding this comment

FrozenGene Aug 6, 2019

Choose a reason for hiding this comment

FrozenGene Aug 6, 2019

Choose a reason for hiding this comment

yzhliu left a comment

Choose a reason for hiding this comment

icemelon commented Aug 6, 2019

anijain2305 commented Aug 6, 2019

yzhliu commented Aug 6, 2019 • edited Loading

tqchen commented Aug 1, 2019 •

edited

Loading

tqchen commented Aug 2, 2019 •

edited

Loading

yzhliu commented Aug 6, 2019 •

edited

Loading