[QNN][Relay] Calling Dialect passes from inside Relay Build API. #3971

anijain2305 · 2019-09-19T06:53:11Z

Background

Qnn is a dialect to support reading pre-quantized models. It has 2 passes - QnnLegalize and QNNCanonicalizeOps. After these 2 passes, we have a graph that only consists of Relay ops. If the original graph did not have any QNN ops, these two passes will keep the graph unmodified.

Problem

We want to have a single API relay.build to compile anything that has been parsed using the framework parsers. However, the QNN passes are currently not in the RelayBuild module. This module shows one way how to do that. The idea is that each Dialect can wrap up their passes into one API and the relay.build can call that API internally.

Alternatives

We can call these two passes between the model parsing and relay.build. But, that does not seem like a good TVM user experience.
Add these two passes in the framework parsers, as QNN is very much associated with parsers. However, QNNLegalize pass needs target info and we cannot (should not) pass target info in the framework parsers.

Relevant Discuss post - https://discuss.tvm.ai/t/rfc-where-to-call-dialect-passes/3951?u=janimesh

@zhiics @tqchen @vinx13 @shoubhik

zhiics · 2019-09-19T15:59:17Z

src/relay/backend/build_module.cc

@@ -224,6 +224,7 @@ class RelayBuildModule : public runtime::ModuleNode {
             const tvm::Target& target_host) {
    targets_ = targets;
    target_host_ = target_host;
+    func = BuildDialect(func, targets);


Instead of doing BuildDialect, we can probably do "DialectOptimize" here as we've already have a module.

https://github.com/dmlc/tvm/blob/af2e2c3f9368478068582862f0e68fd3fe1b506f/src/relay/backend/build_module.cc#L441

zhiics · 2019-09-19T16:12:42Z

@tqchen @vinx13 Do you guys have any thoughts/comments on where should we invoke the optimization for dialects?

vinx13 · 2019-09-19T17:28:28Z

src/relay/qnn/pass/optimize.cc

+
+using transform::Pass;
+
+Function QnnOptimize(const Function& f, const Map<tvm::Integer, tvm::Target>& targets) {


Why not make this function as module->module?

Good point. I will do that.

vinx13 · 2019-09-19T17:29:57Z

@zhiics @anijain2305 Is it possible to integrate these QNN passes into pass manager? Then we can invoke it based on build config

zhiics · 2019-09-19T17:43:15Z

@vinx13 I think we can. But it looks that users are not aware of QNN. They may not be able to feed anything into build_config. What Animesh currently does is to put them under pass manager so that he can pre-invoke it before regular Relay passes. @anijain2305 Am I right?

tqchen · 2019-09-19T17:52:29Z

I think we want to avoid adding indirections into the build function but instead investigate on api composability, see my followup comments on https://discuss.tvm.ai/t/rfc-where-to-call-dialect-passes/3951/3

anijain2305 · 2019-09-19T18:28:14Z

@vinx13 I think we can. But it looks that users are not aware of QNN. They may not be able to feed anything into build_config. What Animesh currently does is to put them under pass manager so that he can pre-invoke it before regular Relay passes. @anijain2305 Am I right?

Yes, you are correct.

anijain2305 · 2019-09-21T00:17:11Z

@tqchen Can you please review if this is what you were thinking about Legalize?

tqchen · 2019-09-23T16:09:32Z

Two following up thoughts:

Make Legalize returning a Pass, just like what we did for other passes in transform.h .
Let us avoid adding dialect.h to the relay header for now and keep things within legalize, as we might still need more time to think about the exact mechanism to register dialect specific passes, perhaps via PackedFunc registry mechanism

anijain2305 · 2019-09-23T16:30:22Z

Two following up thoughts:

Make Legalize returning a Pass, just like what we did for other passes in transform.h .

So, Legalize Pass currently returns a Pass like other transforms - https://github.com/dmlc/tvm/blob/8eb3157aa42f83d03c552328fa7441219df0787e/src/relay/pass/legalize.cc#L93-L99

I am calling Legalize Pass multiple times in build_module Legalize wrapper (I should change the name of method) with different strings. Is that ok?

Let us avoid adding dialect.h to the relay header for now and keep things within legalize, as we might still need more time to think about the exact mechanism to register dialect specific passes, perhaps via PackedFunc registry mechanism

I see. I can do that.

tqchen · 2019-09-23T17:04:48Z

re: pass vs wrapper function: the idea is to create another pass so it can be composed further :)

We want to think a bit more about the API names, especially if it is user facing. In the spirit of minimum user mind burden, perhaps we could only expose the most user friendly API as Legalize(that does not need users to think about dialect)? We could put other API under the namespace of the dialect(e.g. qnn)

anijain2305 · 2019-09-23T22:43:59Z

@tqchen Can you please take a look now?

anijain2305 · 2019-09-25T04:28:37Z

@tqchen I created a Legalize pass under QNN namespace that return a Pass (a sequential pass of QNN related legalizations). Now in BuildRelay, I call this Legalize pass. Is this akin to what you were thinking?

zhiics · 2019-09-26T17:00:53Z

This wrapper now looks good to me. @tqchen Do you have any comment?

anijain2305 · 2019-10-01T16:25:49Z

Ping @tqchen @vinx13 can we try to get some next steps on this?

tqchen · 2019-10-02T22:05:06Z

Approve for now, ideally, it would be great if we can have a transform::Legalize() that users can use which is invariant from any dialect. So we don't have to change the API as we add more dialect.

tqchen · 2019-10-02T22:07:30Z

@anijain2305 specifically, it would be great if we advocate transform::Legalize API while somehow hide qnn::Legalize API(perhaps in a private header)

anijain2305 · 2019-10-02T22:25:53Z

Thanks @tqchen I agree that an ideal transform::Legalize() will be cleaner. Once we have one dialects, I think we will have more clarity about the requirements and design choices. Lets revisit it then.

@zhiics Can you also please review once and assign it to yourself?

zhiics · 2019-10-02T22:39:09Z

Okay. I will merge it for now. We can consider and probably refactor it somehow once there are more dialects come into play.

…che#3971)

* master: (21 commits) [Fix][VM] Fix VM invoke with set_params (apache#4079) [QNN] Refactor fixed point multiplication in requantize (apache#4073) Fix match case in Python-side expr functor (apache#4037) Hide symbols from dependent libraries if HIDE_PRIVATE_SYMBOLS is ON. (apache#4041) Add gradient for log-softmax (apache#4069) [DOC] Fix typos in tutorials (apache#4066) dicrease the complexity of CalcDep from exponential to linear (apache#4053) [Relay][AlterOp] Minor refactor. (apache#4064) [Relay][AlterOp] Improving support for broadcast layout alteration. (apache#4040) Add parses support for zeros_like tflite operator (apache#4042) [Bugfix][TF] reset graph after getting tag of savedmodel (apache#4055) [Relay][VM] Add more passes to VMCompiler (apache#4058) [Relay][VM] Add autotvm context when compile (apache#4062) [Bugfix] Fix target host for vm compiler (apache#4057) [Relay][Training] Add gradient for Crossentropy (apache#3925) [llvm] switch to use Align for llvm trunk (apache#4051) [Relay][TopHub] Add switch to disable TopHub download (apache#4015) [Relay][Op] Add instance norm op (apache#4004) [QNN][Relay] Calling Dialect passes from inside Relay Build API. (apache#3971) [RELAY/PASS] Fix the extent for the post_stmt in the loop partition (apache#3734) ...

zhiics reviewed Sep 19, 2019

View reviewed changes

vinx13 reviewed Sep 19, 2019

View reviewed changes

anijain2305 force-pushed the dialect_pass branch 3 times, most recently from 2ebc769 to 4925ee3 Compare September 21, 2019 00:10

anijain2305 force-pushed the dialect_pass branch from 4925ee3 to 2bc1172 Compare September 21, 2019 00:46

anijain2305 force-pushed the dialect_pass branch 3 times, most recently from 95bd222 to 6f5f53e Compare September 23, 2019 22:42

anijain2305 force-pushed the dialect_pass branch 7 times, most recently from fee4f73 to 6e8129c Compare September 29, 2019 06:28

[QNN][Relay] Calling Dialect passes from inside Relay Build API.

b99687d

anijain2305 force-pushed the dialect_pass branch from 6e8129c to b99687d Compare September 29, 2019 06:30

anijain2305 mentioned this pull request Oct 1, 2019

[QNN][TFLite] Parsing TFLite quantized models. #3900

Merged

tqchen approved these changes Oct 2, 2019

View reviewed changes

zhiics approved these changes Oct 2, 2019

View reviewed changes

zhiics merged commit 36201fe into apache:master Oct 2, 2019

zhiics added the status: accepted label Oct 2, 2019

anijain2305 added a commit to anijain2305/tvm that referenced this pull request Oct 17, 2019

[QNN][Relay] Calling Dialect passes from inside Relay Build API. (apa…

27fd933

…che#3971)

wweic pushed a commit to neo-ai/tvm that referenced this pull request Oct 18, 2019

[QNN][Relay] Calling Dialect passes from inside Relay Build API. (apa…

dcee937

…che#3971)

tqchen mentioned this pull request Nov 8, 2019

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

Closed

anijain2305 deleted the dialect_pass branch November 13, 2019 00:31

anijain2305 restored the dialect_pass branch November 13, 2019 00:31

anijain2305 deleted the dialect_pass branch November 13, 2019 00:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[QNN][Relay] Calling Dialect passes from inside Relay Build API. #3971

[QNN][Relay] Calling Dialect passes from inside Relay Build API. #3971

anijain2305 commented Sep 19, 2019

zhiics Sep 19, 2019

zhiics commented Sep 19, 2019

vinx13 Sep 19, 2019

anijain2305 Sep 19, 2019

vinx13 commented Sep 19, 2019

zhiics commented Sep 19, 2019

tqchen commented Sep 19, 2019

anijain2305 commented Sep 19, 2019

anijain2305 commented Sep 21, 2019

tqchen commented Sep 23, 2019

anijain2305 commented Sep 23, 2019

tqchen commented Sep 23, 2019

anijain2305 commented Sep 23, 2019

anijain2305 commented Sep 25, 2019

zhiics commented Sep 26, 2019

anijain2305 commented Oct 1, 2019

tqchen commented Oct 2, 2019

tqchen commented Oct 2, 2019

anijain2305 commented Oct 2, 2019

zhiics commented Oct 2, 2019


		using transform::Pass;

		Function QnnOptimize(const Function& f, const Map<tvm::Integer, tvm::Target>& targets) {

[QNN][Relay] Calling Dialect passes from inside Relay Build API. #3971

[QNN][Relay] Calling Dialect passes from inside Relay Build API. #3971

Conversation

anijain2305 commented Sep 19, 2019

Background

Problem

Alternatives

zhiics Sep 19, 2019

Choose a reason for hiding this comment

zhiics commented Sep 19, 2019

vinx13 Sep 19, 2019

Choose a reason for hiding this comment

anijain2305 Sep 19, 2019

Choose a reason for hiding this comment

vinx13 commented Sep 19, 2019

zhiics commented Sep 19, 2019

tqchen commented Sep 19, 2019

anijain2305 commented Sep 19, 2019

anijain2305 commented Sep 21, 2019

tqchen commented Sep 23, 2019

anijain2305 commented Sep 23, 2019

tqchen commented Sep 23, 2019

anijain2305 commented Sep 23, 2019

anijain2305 commented Sep 25, 2019

zhiics commented Sep 26, 2019

anijain2305 commented Oct 1, 2019

tqchen commented Oct 2, 2019

tqchen commented Oct 2, 2019

anijain2305 commented Oct 2, 2019

zhiics commented Oct 2, 2019