[Ansor][AutoTVM v2.0] Phase 1: Access Analyzer by merrymercy · Pull Request #6103 · apache/tvm

merrymercy · 2020-07-21T03:49:16Z

For the full upstream plan, see Ansor RFC.

This pr

Introduces the access analyzer which analyzes the read-write relations in a compute declaration.
The search policy will use the analysis results to make decisions such as doing multi-level tiling for an op or strictly inlining an op.
Moves ansor related header files to include/tvm/auto_scheduler and polishes some comments.

merrymercy · 2020-07-21T03:52:59Z

cc @junrushao1994 @comaniac @jcf94 @FrozenGene @tqchen

junrushao · 2020-07-21T04:43:48Z

It is one of the core part of the system. Will take a look later tomorrow night.

merrymercy · 2020-07-21T14:55:05Z

This PR is ready for review. But It has some dependency on #6073. I will make it pass the ci test after #6073 is merged

jcf94 · 2020-07-21T15:13:47Z

This PR is ready for review. But It has some dependency on #6073. I will make it pass the ci test after #6073 is merged

#6073 's macOS build failure seems caused by some infrastructure error, I think it's ready for merge? I'll continue to upstream the left steps after #6073 .

junrushao · 2020-07-22T04:34:18Z

+ /*! \brief Store whether the operation is an output operation */
+ OperationMap<bool> is_output;
+ /*! \brief Store the topological order of operations */
+ Array<te::Operation> ops_topo_order;


What's the relationship between this array and ComputeDAG::ops?

They are the same. I store it in AccessAnalyzer because it is used first here.
In the constructor of ComputeDAG, it copies ops_topo_order as its ops

junrushao · 2020-07-22T06:38:30Z

+namespace auto_scheduler {
+
+/*! \brief Static analysis result for a ComputeDAG */
+class AccessAnalyzerNode : public Object {


I feel like AccessAnalyzer itself can be a much more principled and extensible component of the system, so shall we put it in a separate file instead?

Agree. Maybe have an analysis.h to expect more analyzers in the future. On the other hand, another direction might be renaming AccessAnalyzer to ComputeDAGAnalyzer, because it provides some APIs for the ops in a compute DAG, such as NeedMultiLevelTiling, IsOutput, etc.

tqchen · 2020-07-22T16:05:21Z

cc @jwfromm @mbrookhart @jroesch

junrushao · 2020-07-22T07:01:07Z

 fail_ = true;
 break;
 }
+ cur_type_code_ = pop->output_dtype(0).code();


could you elaborate why we need cur_type_code_? how do we deal with the case that computation is mixed with int8 and fp32?

The FLOP information is only used for printing and debugging. It is okay to just give a rough estimation.

@merrymercy I agree that it is totally okay if we can just use rough information (in fact it is highly non-trivial to get accurate info without backend info). My point is that cur_type_code_ comes from the dtype of output, but it is totally possible that a compute dag contains computation of different type code (int8, fp16)

It is not common at all. The common case is either int8->int16/32 or fp16/fp16->fp32

comaniac

Overall LGTM. Agree with @junrushao1994 on most comments.

comaniac · 2020-07-22T18:24:56Z

+namespace auto_scheduler {
+
+/*! \brief Static analysis result for a ComputeDAG */
+class AccessAnalyzerNode : public Object {


Agree. Maybe have an analysis.h to expect more analyzers in the future. On the other hand, another direction might be renaming AccessAnalyzer to ComputeDAGAnalyzer, because it provides some APIs for the ops in a compute DAG, such as NeedMultiLevelTiling, IsOutput, etc.

junrushao · 2020-07-22T19:38:27Z

+ node->read_from[op] = OperationMap<std::vector<std::vector<PrimExpr>>>();
+ } else if (auto cop = op.as<te::ComputeOpNode>()) {
+ TensorAccessExtractor extractor;
+ for (const auto& exp : cop->body) {


just curious: did we test with ComputeOp with multiple bodies?

No. It will be addressed later.

junrushao

Finally done this round of review... Thanks for the contribution!

junrushao · 2020-07-22T19:57:24Z

+ const Array<PrimExpr>& output_shape = op->output_shape(0);
+ const Array<PrimExpr>& producer_shape = producer->output_shape(0);


Do it only work for te::Operation with a single output? Do we have a fallback solution for operators with multiple outputs like argmax?

Even with multiple outputs, the shape will be the same

merrymercy · 2020-07-23T23:13:31Z

@junrushao1994 @jcf94 @comaniac Most of the comments are addressed. I added more doc and make the name convention more consistent and meaningful. Please take another look

junrushao

LGTM

comaniac

LGTM. Thanks.

* add access analyzer * add test cases * move header files and polish comments * fix lint * update * fix lint * address comments * fix lint

merrymercy commented Jul 21, 2020
View reviewed changes

Comment threadsrc/auto_scheduler/compute_dag.h

jcf94 reviewed Jul 21, 2020
View reviewed changes

Comment threadtests/python/unittest/test_auto_scheduler_common.py Outdated

tqchen added the status: need review label Jul 21, 2020

merrymercy added 2 commits July 21, 2020 17:55

add access analyzer
cf84177

add test cases
3a8b4b4

merrymercy force-pushed the pr-access-analyzer branch 2 times, most recently from ea153e2 to 56b0187Compare July 22, 2020 05:25

move header files and polish comments
966c3cc

merrymercy force-pushed the pr-access-analyzer branch from 95ef46c to 966c3ccCompare July 22, 2020 05:54

fix lint
1b0f693

FrozenGene approved these changes Jul 22, 2020
View reviewed changes

merrymercy added 2 commits July 21, 2020 23:16

update
64244ab

fix lint
bea7dcf

junrushao reviewed Jul 22, 2020
View reviewed changes

comaniac approved these changes Jul 22, 2020
View reviewed changes

junrushao reviewed Jul 22, 2020
View reviewed changes

junrushao requested changes Jul 22, 2020
View reviewed changes

merrymercy force-pushed the pr-access-analyzer branch 2 times, most recently from 99e9ab3 to b01fcf8Compare July 23, 2020 23:08

merrymercy force-pushed the pr-access-analyzer branch from b01fcf8 to f63929bCompare July 23, 2020 23:15

junrushao approved these changes Jul 23, 2020
View reviewed changes

jcf94 approved these changes Jul 24, 2020
View reviewed changes

address comments
c690c32

merrymercy force-pushed the pr-access-analyzer branch from f63929b to c690c32Compare July 25, 2020 00:03

comaniac approved these changes Jul 25, 2020
View reviewed changes

merrymercy force-pushed the pr-access-analyzer branch from 4cfe0cf to e9933c6Compare July 25, 2020 00:14

fix lint
e032686

merrymercy force-pushed the pr-access-analyzer branch from e9933c6 to e032686Compare July 25, 2020 00:18

merrymercy merged commit c9cbd04 into apache:masterJul 25, 2020

merrymercy mentioned this pull request Jul 25, 2020
[Ansor][AutoTVM v2.0] Phase 1: Add cache_read/cache_write steps #6107
Merged

tqchen added status: accepted and removed status: need review labels Jul 25, 2020

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Aug 26, 2020
[Ansor][AutoTVM v2.0] Phase 1: Access Analyzer (apache#6103)
245df85
* add access analyzer * add test cases * move header files and polish comments * fix lint * update * fix lint * address comments * fix lint

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Aug 26, 2020
[Ansor][AutoTVM v2.0] Phase 1: Access Analyzer (apache#6103)
d2b7354
* add access analyzer * add test cases * move header files and polish comments * fix lint * update * fix lint * address comments * fix lint

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Aug 26, 2020
[Ansor][AutoTVM v2.0] Phase 1: Access Analyzer (apache#6103)
637589a
* add access analyzer * add test cases * move header files and polish comments * fix lint * update * fix lint * address comments * fix lint

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Sep 2, 2020
[Ansor][AutoTVM v2.0] Phase 1: Access Analyzer (apache#6103)
f369287
* add access analyzer * add test cases * move header files and polish comments * fix lint * update * fix lint * address comments * fix lint

trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Sep 3, 2020
[Ansor][AutoTVM v2.0] Phase 1: Access Analyzer (apache#6103)
b4aa451
* add access analyzer * add test cases * move header files and polish comments * fix lint * update * fix lint * address comments * fix lint

ZihengJiang mentioned this pull request Sep 25, 2020
TVM v0.7 Release Note Candidate #6486
Closed

merrymercy deleted the pr-access-analyzer branch September 27, 2020 00:52

		const Array<PrimExpr>& output_shape = op->output_shape(0);
		const Array<PrimExpr>& producer_shape = producer->output_shape(0);

Uh oh!

Conversation

merrymercy commented Jul 21, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

merrymercy commented Jul 21, 2020

Uh oh!

Uh oh!

junrushao commented Jul 21, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

merrymercy commented Jul 21, 2020

Uh oh!

jcf94 commented Jul 21, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

merrymercyJul 22, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tqchen commented Jul 22, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

merrymercyJul 22, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

comaniac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

junrushao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

merrymercyJul 23, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

merrymercy commented Jul 21, 2020•
edited
Loading

junrushao commented Jul 21, 2020•
edited
Loading

jcf94 commented Jul 21, 2020•
edited
Loading

merrymercyJul 22, 2020•
edited
Loading

merrymercyJul 22, 2020•
edited
Loading

merrymercyJul 23, 2020•
edited
Loading