Repository navigation
[SCHEDULE] New Reduction Mode for Tensorize - #727
Conversation
tqchen
left a comment
tqchen
left a comment
There was a problem hiding this comment.
there are some codestyle problem, you can reproduce locally via make lint. Other comments are explicitly inlined
| Stmt body, | ||
| Stmt update) | ||
| { | ||
| Expr condition = (make_zero(Int(32)) == make_zero(Int(32))); //will try Bool(1) |
There was a problem hiding this comment.
build a Array<Expr> containing condition, use https://github.com/dmlc/tvm/blob/master/src/arithmetic/compute_expr.h#L42 to do reduction
| auto vit = dom_map.find(iv); | ||
| CHECK(vit != dom_map.end()); | ||
| const Range& vrange = vit->second; | ||
| Expr newcond = ( iv->var == vrange->min); |
There was a problem hiding this comment.
use condition
if (likely(var1> min1) or likely(var2 > min2)) {
update
} else {
body
}
| return ir::Substitute(s, init); | ||
| } | ||
|
|
||
| Stmt TransformUpdate(const Stage& stage, |
There was a problem hiding this comment.
This function is specific to Tensorize, move it to tensorize.cc, no need to keep comment in header file, move comment document to tensorize.cc as well
There was a problem hiding this comment.
done, move the function into tensorize.cc
| update = MergeNest(update_nest, update); | ||
| return MergeNest(common, Block::make(init, update)); | ||
| } else { | ||
| // The update |
There was a problem hiding this comment.
Add a comment about what this logic is about. Something like
// when init intrinsic is not available, transform to reset in the first iter.
| return MergeNest(common, Block::make(init, update)); | ||
| } else { | ||
| // The update | ||
| Stmt update = TransformUpdate (stage, dom_map, |
tqchen
changed the title
|
Please add a unittestcase to https://github.com/dmlc/tvm/blob/master/tests/python/unittest/test_schedule_tensorize.py#L1 |
|
there is one additional drawback of the approach which requires additional verification. We cannot let the For example, if condition contains rc>1 (and rc is from [0, n) ), then the initial checking condition is no longer correct. Error shall be raised when such case is detected. This can be done via something similar to https://github.com/dmlc/tvm/blob/master/src/op/tensorize.cc#L124 |
| CHECK(intrin->reduce_init.defined()) | ||
| << "Reduction init op for intrin " << intrin << " is not defined"; | ||
| // Comment out the following check for the case when there is no init func | ||
| //CHECK(intrin->reduce_init.defined()) |
|
Refined the code per review comment, and add a test case in unittest, will add more cases in the following days. |
| @@ -0,0 +1,90 @@ | |||
| import tvm | |||
|
|
|||
There was a problem hiding this comment.
move this to test_schedule_tensorize.py We can use different function name
* when there is no intrin func, using body for initialization. For issue 714. * Refine code per review comments, and add a test case. * Fix lint issues.
* when there is no intrin func, using body for initialization. For issue 714. * Refine code per review comments, and add a test case. * Fix lint issues.
* when there is no intrin func, using body for initialization. For issue 714. * Refine code per review comments, and add a test case. * Fix lint issues.
2 participants
…e 714. @tqchen , This fix can resolve the original issue which I raised. Please review it , thanks.